Skip to content

Conversation

Hywan
Copy link

@Hywan Hywan commented Mar 5, 2019

This patch first updates the documentation to compile the Rust example.

Then this patch updates the Rust code, since panic_implementation is
a deprecated feature. Also extern is similar to extern "C". Less
code is always better.

Finally, this patch compiles and optimises the Rust Wasm binary file
(with a simple wasm-gc run). The final binary size is 1.4Kb.

I let you run the benchmark if necessary. I don't have the same setup so results will be too different.

This patch first updates the documentation to compile the Rust example.

Then this patch updates the Rust code, since `panic_implementation` is
a deprecated feature. Also `extern` is similar to `extern "C"`. Less
code is always better.

Finally, this patch compiles and optimises the Rust Wasm binary file
(with a simple `wasm-gc` run). The final binary size is 1.4Kb.
@@ -9,4 +9,4 @@ crate-type = ["cdylib"]

[profile.release]
lto = true
opt-level = 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should stay '3' instead 's' because AS also using -O3 so for better comparison they should have same opt levels

@@ -42,6 +42,6 @@ Benchmark
| **AssemblyScript WASM** | **2901** | **2** |
| AssemblyScript ASMJS | 3720 | 19* |
| JavaScript | 2716 | 5* |
| Rust WASM | 2883 | 13 |
| Rust WASM | 2883 | 1.4* |
Copy link
Member

@MaxGraey MaxGraey Mar 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust doesn't produce js glue code in current case so mark "*" is unnecessary

@@ -1,7 +1,7 @@
### Build

```bash
cargo build --release --target=wasm32-unknown-unknown
cargo +nightly build --release --target wasm32-unknown-unknown
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's nice. I'm using nightly by default and totally forgot that others use stable or beta by default.

@MaxGraey
Copy link
Member

MaxGraey commented Mar 5, 2019

Thanks! Close to favour #528

@MaxGraey MaxGraey closed this Mar 5, 2019
@Hywan
Copy link
Author

Hywan commented Mar 5, 2019

Why closing the PR? I would have appreciated to update the PR myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants