Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Benchmark Project #67

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

martindevans
Copy link
Contributor

Added a benchmark project. Currently only one benchmark which compares performance to wasmtime, but it's easy to add more. This project actually beats wasmtime!

The benchmark runs a function called bench, which is adapted from a random Rust project I had lying around (calculating various rocket nozzle parameters). I'm happy to share the code for this if you want, but I'm not sure what the best way to do it is - would you just want the entire Rust project included in the wasm folder?

Copy link
Owner

@RyanLamansky RyanLamansky left a comment

Choose a reason for hiding this comment

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

A 1.42 MB binary goes against my goal to keep this repo extremely lean--it's bigger than all of the already-included WASMs combined.

I like the idea of a benchmark in principal, though. Ideally something like this would ideally be expressed as code generated using the library itself to avoid having to check in any more binaries.

…s performance to wasmtime. This project actually beats wasmtime!

The benchmark runs a function called `bench`, which is adapted from a random Rust project I had lying around (calculating various rocket nozzle parameters). I'm happy to share the code for this if you want, but I'm not sure what the best way to do it is - would you just want the entire Rust project included in the wasm folder?
@martindevans
Copy link
Contributor Author

I've changed the Rust compile options to optimise for size, using:

[profile.release]
lto = true
panic = "abort"
opt-level = "s"

Which has reduced the file size down to 167KB. Is that more acceptable? I would prefer to use a "real world" test file if at all possible, rather than something hand written, because I think that leads to a more realistic benchmark result.

@RyanLamansky
Copy link
Owner

Yeah, that size is a lot better. I'm not sure if I want to merge this, though: it's adding maintenance surface area to a project I only barely have enough time to keep alive. I'm definitely not trying to compete with wasmtime.

Maybe a blog or (if you're particularly ambitious) a Youtube channel might be more appropriate?

@martindevans
Copy link
Contributor Author

My motivation in adding it is that it can be used to drive future optimisations. Benchmarks can be added to cover a specific area and then that's a good starting pointing for profiling and optimising performance of that area.

Personally I think it is worth including, there should be very little maintenance overhead, but that's your call of course! I can use it locally for optimisations whether or not it's merged, so I'm not too worried if you want to close this PR :)

I actually didn't intend to PR the wasmtime comparison benchmark initially. It was just an easy one to put together and I was curious how a relatively simple translation to IL would compare to the much more complex wasmtime project. I fully expected wasmtime to win. I only polished it up to make the PR because of the unexpected result!

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