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

Wasm interp speedup #4707

Closed
wants to merge 4 commits into from
Closed

Wasm interp speedup #4707

wants to merge 4 commits into from

Conversation

brian-carroll
Copy link
Contributor

@brian-carroll brian-carroll commented Dec 7, 2022

See commit messages for Zig test timing numbers.

@brian-carroll
Copy link
Contributor Author

So far the winner seems to be the most naïve approach: Vec<Value>!
It does the Zig tests in 124ms as opposed to 132ms for the "unaligned byte vector" idea.

This makes sense if:

  • the bottleneck is the conversion logic in push and pop (we often want to get a Value out anyway)
  • inefficient memory usage doesn't matter because stack access patterns mean it's almost always always in cache

BUT actually that first bullet point isn't quite right because a lot of instructions use pop_i32 and friends. So in most cases it shouldn't matter if the item in the vector is exactly a Value.

I also want to try splitting the type discriminant and the data into separate vectors and just see what happens.

@folkertdev
Copy link
Contributor

on my machine the results aren't that convincing, really. None of the gains are statistically significant agains the baseline (the branch we merged yesterday evening)

folkertdev@folkertdev ~/r/r/c/c/b/bitcode ((890ef512…))> 

Benchmark #1: ~/roc/roc/target/release-with-debug/baseline src/zig-cache/o/b9ad7c1f8d1a1ca23dddd079fbf65140/test.wasm "foo"
  Time (mean ± σ):      89.1 ms ±   3.1 ms    [User: 87.9 ms, System: 1.3 ms]
  Range (min … max):    86.8 ms … 103.9 ms    28 runs
 
Benchmark #2: ~/roc/roc/target/release-with-debug/roc_wasm_interp src/zig-cache/o/b9ad7c1f8d1a1ca23dddd079fbf65140/test.wasm "foo"
  Time (mean ± σ):      88.0 ms ±   1.2 ms    [User: 85.7 ms, System: 2.4 ms]
  Range (min … max):    86.2 ms …  91.4 ms    33 runs
 
folkertdev@folkertdev ~/r/r/c/c/b/bitcode ((ec7490c5…))> 

Benchmark #1: ~/roc/roc/target/release-with-debug/baseline src/zig-cache/o/b9ad7c1f8d1a1ca23dddd079fbf65140/test.wasm "foo"
  Time (mean ± σ):      89.0 ms ±   3.4 ms    [User: 87.5 ms, System: 1.6 ms]
  Range (min … max):    86.6 ms … 103.4 ms    28 runs
 
Benchmark #2: ~/roc/roc/target/release-with-debug/roc_wasm_interp src/zig-cache/o/b9ad7c1f8d1a1ca23dddd079fbf65140/test.wasm "foo"
  Time (mean ± σ):      88.2 ms ±   1.1 ms    [User: 86.9 ms, System: 1.4 ms]
  Range (min … max):    86.8 ms …  92.0 ms    33 runs

To me that means that the profile isn't really all that helpful here.

@folkertdev
Copy link
Contributor

I tried using the LEB functions from https://github.com/nnethercote/rust/blob/ad7802f9d45b884dad58931c7a8bec91d196ad0e/src/libserialize/leb128.rs (those were optimized, see rust-lang/rust#69050 (comment)) but didn't really see an effect.

@brian-carroll brian-carroll deleted the wasm_interp_speedup branch December 17, 2022 21:46
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