Fix installing cargo-binstall in do_main_binstall

[NobodyXu]

Signed-off-by: Jiahao XU [email protected]

[alsuren]

@NobodyXu do you want to run a patch release for this?

[NobodyXu]

@NobodyXu do you want to run a patch release for this?

I was actually thinking of replacing tiny-json with serde-json + serde + wa-serde-derive to see if I can make fetching the version more efficient without significantly hit compile time.

If you don't like that proposal at all, then we should go ahead and have a patch release.

[alsuren]

make fetching the version more efficient without significantly hit compile time

Do you have performance numbers showing that tiny-json parsing is slower than the curl call to crates.io or the curl | tar to unpack the binary? This feels surprising to me. I would expect the json parsing to be basically insignificant.

[NobodyXu]

make fetching the version more efficient without significantly hit compile time

Do you have performance numbers showing that tiny-json parsing is slower than the curl call to crates.io or the curl | tar to unpack the binary? This feels surprising to me. I would expect the json parsing to be basically insignificant.

Last I checked https://crates.io/api/v1/crates/$name would download somewhere around 50k bytes of JSON.

Since tiny-json parses every object/array/string, it's going to incur quite a lot allocation, where as ssrde-jsonc combined with derive would skip most of them and would only deserialise the field max_stable_version.

[alsuren]

When deciding where to spend time optimising, please consider https://en.wikipedia.org/wiki/Amdahl%27s_law . Get the breakdown for time spent on each bit and then make a guess at what your % speed-up will be and what that translates to for the whole run.

Also remember that for CI use-cases, cargo install cargo-quickinstall && cargo quickinstall $package is intended to be used as a drop-in replacement for cargo install $package. cargo-quickinstall is only used once for this use-case and then thrown away. If cargo-binstall is already installed, we may never hit the json-decoding codepath.

[alsuren]

$ time curl https://crates.io/api/v1/crates/cargo-binstall > binstall.json
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 48987  100 48987    0     0  62384      0 --:--:-- --:--:-- --:--:-- 62563

real    0m0.806s
user    0m0.022s
sys     0m0.022s

$ time jq . binstall.json > /dev/null

real    0m0.042s
user    0m0.031s
sys     0m0.011s

0.042[1] / 0.806[2] is 5%. This doesn't feel like something that's worth spending time optimising.

[1] Assuming that jq is vaguely representitive of json parsing times, and that tiny-json will be in the same ballpark.
[2] Admittedly I'm on a train at the moment so your curl performance might be better than mine.

[alsuren]

I've just given you a bunch of reasons not to do it, but don't let that stop you.

If the reason for switching to serde-json + serde + wa-serde-derive is because it's cooler or more maintainable then I can get behind that (I would actually love to see what happens to our cargo clean && cargo build --release --jobs=1 times when we do this, because wa-serde-derive is the coolest thing ever. I would also love to see a wasm32-wasi target added to cargo-quickinstall someday, because it wasi is the new hotness).

[NobodyXu]

Yeah I think we should at least try wa-serde-derive out and see how it affect compile-time and runtime performance.

I would also love to see a wasm32-wasi target added to cargo-quickinstall someday, because it wasi is the new hotness

I'm also thinking about this.
Since cargo-quickinstall does not use an async/network but only runs processes, I think it's relatively easy to compile cargo-quickinstall to wasi without any extension.

[alsuren]

I'm also thinking about this. Since cargo-quickinstall does not use an async/network but only runs processes, I think it's relatively easy to compile cargo-quickinstall to wasi without any extension.

I hadn't thought about that. I was more imagining making a version of `bat` or `ripgrep` in wasi that we could use as a universal fallback (to replace -musl). How would you bootstrap a wasm version of cargo-quickinstall, and what would you use it for?

…

[NobodyXu]

I hadn't thought about that. I was more imagining making a version of bat
or ripgrep in wasi that we could use as a universal fallback (to replace
-musl).

We've also considered this in cargo-binstall cargo-bins/cargo-binstall#246

TL;DR is we might want to include a wasm interpreter/compiler in cargo-binstall, or checks whether the users' system supports running wasi (e.g. binfmt-ext on linux), or just use wasmer.

But it will be a good start for quickinstall to build binaries for wasi and then upload it to the release.

How would you bootstrap a wasm version of cargo-quickinstall, and what
would you use it for?

Could be for places where we do not provide pre-built artifacts but have wasm interpreter?
Not sure about its use cases though since building quickinsrall from source is also very fast.