Wasm opt rust

[zetanumbers]

• lazily build tasks;
• site documentation;
• cargo xtask run print output cartridges;
• prompt to cache wasm-opt;

[zetanumbers]

@aduros

[FaberVitale]

I've played a bit but I've not tried the download feature because I've got already wasm-opt on PATH.
Overall it works and is pretty nifty but I've got few observations.

unimplemented! if Os is not supported

Can we use binaryen.js if a binary is not available for the user machine instead of panicking?
Note that there are currently issues with M1 macs: WebAssembly/binaryen#4334

Too much code in xtask-build directory

There's probably too much logic inside xtask/xtask-build.
Is it possible to move most of the logic in an external lib a leave there the bare minimum?

Readme

This branch is a bit behind main:
I'd like to see how your changes interact with the Readme feature before merging this PR.

---

I think that is important that we get more feedback on this overhaul.
Maybe @claudiomattera could chime in on this one?

[zetanumbers]

unimplemented! if Os is not supported

Can we use binaryen.js if a binary is not available for the user machine instead of panicking? Note that there are currently issues with M1 macs: WebAssembly/binaryen#4334

It seems to me. There is. Another reason for. WASM-4 SDK.

Too much code in xtask-build directory

There's probably too much logic inside xtask/xtask-build. Is it possible to move most of the logic in an external lib a leave there the bare minimum?

I had a thought about implementing cargo-wasm4 for my wasm4-rs repo. I could implement this thing inside of the cargo-wasm4 and expose it as a library. Optimization like --zero-filled-memory is runtime implementation dependent, so i don't think i could expand it to more general case. However i am worried @aduros won't approve usage of 3rd party crate there anyway.

[claudiomattera]

Maybe @claudiomattera could chime in on this one?

First of all, let me remind everyone that I have little experience with WASM, so do not take my opinions too seriously :D

Anyway, it is nice to see someone working on streamlining WASM-4 games development with Rust.
I was quite confused when I started playing with this console and could not understand the reason for cartridges being so oversized.
We need a way to include wasm-opt in the build process transparently to the user, so that the final cartridge does not contain debug information.

I also have a few observations:

1. I do not like too much that it overhauls the build process by wrapping everything in a new command.
   Rust developers might expect a familiar workflow involving cargo build --release and build upon that.

2. I also do not like the workspace structure, where crates contain other crates.
   I would probably make the top-level Cargo.toml only a workspace, and create three subdirectories cartridge, xtask and xtask-build with the actual crates.

3. I am conflicted whether downloading Binaryen if it is missing is a good idea or not.
   First of all, depending on binary-install brings quite a lot of build-time dependencies.
   I really liked that the default build of a Rust template has zero dependencies (one, if you enable buddy-alloc).

   binary-install also depends on cURL and OpenSSL.
   I wonder how that would turn out on systems where those tools do not exist or are difficult to build, such as MUSL-based Linux systems or architectures different than x86[_64] and ARM.
   But realistically this might not be an actual issue for anyone.

   On top of that, crate binary-install seems neither mature nor actively developed.
   Latest stable version 0.0.2 is from three years ago, latest unstable version 0.0.3.alpha1 is from over one year ago, and last activity on the repo is also quite old.
   Aside from the version number, its readme on crates.io is basically empty.
   It makes it a bit difficult to figure out what it does and how it works.

   So I would at least make the download optional, perhaps hiding it behind a Cargo feature.
   That would remove binary-install and thin out the dependency tree.

   I would also verify that no unused features of other dependencies (cargo_metadata and log) are enabled.
   That might remove a couple more dependencies.

4. Finally, I am not a big fan of stripping all debug information from the release cartridge.
   I use twiggy quite a lot to investigate which functions take space in the cartridge, but it needs debug information, otherwise it will just tell me something like

    Shallow Bytes │ Shallow % │ Item
   ───────────────┼───────────┼────────────────
             7831 ┊    36.13% ┊ code[0]
             4168 ┊    19.23% ┊ data[0]
             2344 ┊    10.82% ┊ data[1]
             1166 ┊     5.38% ┊ code[1]
              782 ┊     3.61% ┊ code[2]
   

   which is quite useless.
   I would prefer to leave the unstripped cartridge around.

   But now that I read it again, I realize I can just call cargo build --release without using the wrapper, so it is not an actual issue.

I hope I did not sound too negative, though.
Extracting the path of build artefacts from Cargo's output was quite clever (I normally just assume they are in target/wasm32-unknown-unknown/release, but this seems more robust).
Ultimately it does generate a properly stripped and optimized cartridge, perhaps it can be a good starting point for the Rust template.

In comparison, I took a much less ambitious approach in my projects.
I simply created a Makefile.toml (using cargo-make), and defined few tasks to automate the build process.

cargo make build-release builds the cartridge leaving all debug information, and cargo make build-optimized also runs wasm-opt on it.
I also have other useful tasks such as cargo make run-optimized-native, or cargo make bundle-into-cartridge, or even cargo make verify-cartridge-size (useful in pipelines).

[zetanumbers]

I do not like too much that it overhauls the build process by wrapping everything in a new command.
Rust developers might expect a familiar workflow involving cargo build --release and build upon that.

There's rust-lang/cargo#545, but it is not resolved.

I also do not like the workspace structure, where crates contain other crates.
I would probably make the top-level Cargo.toml only a workspace, and create three subdirectories cartridge, xtask and xtask-build with the actual crates.

There's nothing wrong with it. This PR is modelled after https://github.com/matklad/cargo-xtask/ repo.

3. binary-install is wasm-pack's dependency and is under rustwasm org. So it's not a random crate and it has some support. I should investigate openssl and curl support tho.

I would also verify that no unused features of other dependencies (cargo_metadata and log) are enabled.
That might remove a couple more dependencies.

I remember i have done that already. There's already default-features = false on env_logger for example.

4. We could add a custom flag.

(5.) I have considered cargo-make, but there was cargo-xtask which works out of the box. Also there's no cargo_metadata for cargo-make.

EDIT: i've eased my language here a bit. Sorry for being rude to you @claudiomattera and thank you for your review. I have spent a lot of time on user friendliness, so i was stressed there a bit.

[zetanumbers]

I am no longer wanna do anything before there's any consensus on a solution. I had idea for cargo-wasm4 described above, but i am not gonna do it, for it to be disapproved right after.

[claudiomattera]

I invite you @claudiomattera to go ahead and solve rust-lang/cargo#545

That feature looks nice, it would open up a few possibilities of automation without relying on external tools.
Hopefully its design will be completed sometime, and we will get the dual of build.rs scripts.

There's nothing wrong with it outside of your preference.

Correct.

binary-install is wasm-pack's dependency and is under rustwasm org. So it's not some random crate.

It does not really rebut any of my points, though.

It carries lots of dependencies, including system ones (cURL and OpenSSL).
It has not seen any stable release in 3 years.
It has not even hit 0.1.0 "milestone", sign that the authors might consider significantly changing its interface.
It is poorly documented: the readme is almost empty, the rustdoc is only slightly better (e.g. it does not state where files are downloaded).

I would personally try to avoid that kind of direct dependency.

Why are you assuming i haven't done that already? There's already default-features = false on env_logger for example.

That was more of a reminder than anything else.

I have been auditing some of my own projects lately, and I always found some unused features in my dependencies.
Good to know that you already took care of that.

We could add a flag so whatever.

Yes, that would a good idea.
My main objection to this approach is depending on binary-install, which is only used for downloading wasm-opt if missing.
If that was tied to a Cargo feature, users could easily disable it (I would personally keep it opt-in, though, and not opt-out).

(5.) I have considered cargo-make, but there was cargo-xtask which works out of the box. Also there's no cargo_metadata for cargo-make.

Sure, I just mentioned it as a simpler, less powerful alternative.
My cargo-make tasks are all manually written, while your approach seems more automated.

---

By the way, I was wondering if this step is something that could be embedded into the linker and Cargo's --strip option.
As I said, I am not too experienced with WASM, and I am not sure how the linker works for that target, but after all we are stripping the final object code.
Is there a way to make Cargo call wasm-opt in that way?

[zetanumbers]

By the way, I was wondering if this step is something that could be embedded into the linker and Cargo's --strip option.
As I said, I am not too experienced with WASM, and I am not sure how the linker works for that target, but after all we are stripping the final object code.
Is there a way to make Cargo call wasm-opt in that way?

Well there's strip = true cargo option which is coming to 1.59.0. But that wouldn't solve everything tho. Allocator uses statically allocated heap, which results in a buch of zeroes in the final binary (~13KiB was for me i think). wasm-opt -Oz --zero-filled-memory solves this.

With regards to the binary-install. I think it's not an ideal dependency, but i am afraid we cannot surpass it's quality by our own. I only guess WASM4_SDK would help with that, by shipping wasm-opt in it.

Again i would like to apologise for my rude behaviour @claudiomattera. Your thoughts on the subject are valuable here, do not doubt that.

[claudiomattera]

Well there's strip = true cargo option which is coming to 1.59.0. But that wouldn't solve everything tho. Allocator uses statically allocated heap, which results in a buch of zeroes in the final binary (~13KiB was for me i think). wasm-opt -Oz --zero-filled-memory solves this.

Yeah.
In my previous pull request #138 I replaced those static muts with raw pointes, but @aduros preferred to keep them that way and strip the executable with wasm-opt instead (it would have only solved the problem for the allocator, anyway, and not for other uses of statically-allocated heap, so it definitely made sense).

With regards to the binary-install. I think it's not an ideal dependency, but i am afraid we cannot surpass it's quality by our own. I only guess WASM4_SDK would help with that, by shipping wasm-opt in it.

Fair point.
That crate might be young and unstable, but reimplementing it from scratch would not solve anything.

Again i would like to apologise for my rude behaviour @claudiomattera. Your thoughts on the subject are valuable here, do not doubt that.

No problem, and of course your work on this topic is also appreciated :)