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

Optimize host calls in the wasmi interpreter #291

Merged
merged 5 commits into from
Jan 3, 2022

Conversation

Robbepop
Copy link
Member

@Robbepop Robbepop commented Dec 29, 2021

This commit does two things to speed-up host calls:

  • Reuse heap allocations for consecutive host calls to hold arguments.
  • More efficient copying of stack values to host argument buffer.

This optimization was carved out from #287 and it would be nice to have it included so that both v0 and v1 implementation have a more comparable performance once wasmi benchmarks are more precise.

Benchmarks

No noticeable performance impact was measured, however, that's mainly because the current set of benchmarks do not cover the particular scenario of performing lots of host function calls.

Before

test bench_regex_redux ... bench:   1,831,339 ns/iter (+/- 153,861)
test bench_rev_comp    ... bench:   1,661,062 ns/iter (+/- 146,230)
test bench_tiny_keccak ... bench:   1,276,610 ns/iter (+/- 132,734)
test fac_opt           ... bench:      23,268 ns/iter (+/- 4,547)
test fac_recursive     ... bench:      24,568 ns/iter (+/- 1,908)
test recursive_ok      ... bench:     615,655 ns/iter (+/- 43,826)
test recursive_trap    ... bench:      79,457 ns/iter (+/- 4,446)

After

test bench_regex_redux ... bench:   1,802,265 ns/iter (+/- 130,377)
test bench_rev_comp    ... bench:   1,656,515 ns/iter (+/- 126,081)
test bench_tiny_keccak ... bench:   1,260,390 ns/iter (+/- 118,418)
test fac_opt           ... bench:      22,251 ns/iter (+/- 1,458)
test fac_recursive     ... bench:      23,990 ns/iter (+/- 2,073)
test recursive_ok      ... bench:     593,989 ns/iter (+/- 86,742)
test recursive_trap    ... bench:      78,235 ns/iter (+/- 5,871)

This commit does two things to speed-up host calls:
- Reuse heap allocations for consecutive host calls to hold arguments.
- More efficient copying of stack values to host argument buffer.
@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2021

Codecov Report

Merging #291 (3b9109b) into master (9556783) will decrease coverage by 0.45%.
The diff coverage is 42.59%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
- Coverage   80.02%   79.56%   -0.46%     
==========================================
  Files          30       31       +1     
  Lines        4696     4738      +42     
==========================================
+ Hits         3758     3770      +12     
- Misses        938      968      +30     
Impacted Files Coverage Δ
benches/src/lib.rs 0.00% <0.00%> (ø)
src/runner.rs 92.30% <85.18%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9556783...3b9109b. Read the comment docs.

@Robbepop
Copy link
Member Author

Added benchmarks in 1455e9a.

Run the benchmark using:

cargo bench --manifest-path benches/Cargo.toml host_call

Before

running 1 test
test host_calls        ... bench:     135,455 ns/iter (+/- 19,820)

After

running 1 test
test host_calls        ... bench:      95,340 ns/iter (+/- 13,254)

So we can measure a speed-up of roughly 42% for the benchmark.

/// # Panics
///
/// If the value stack does not have at least `depth` stack entries.
pub fn pop_as_slice(&mut self, depth: usize) -> &[RuntimeValueInternal] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest removing the as here. In my mind that means returning the whole thing as slice and not only parts of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion, will do this!

@Robbepop Robbepop merged commit f3bb8d6 into master Jan 3, 2022
@athei athei deleted the rf-optimize-host-calls branch January 3, 2022 14:58
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.

3 participants