search

wasmi-labs / wasmi

WebAssembly (Wasm) interpreter.
https://wasmi-labs.github.io/wasmi/
Apache License 2.0
1.56k stars 281 forks source link

Use `CachedMemory` in bulk-ops executors #1075

Closed Robbepop closed 3 months ago

Robbepop commented 3 months ago

Locally this yielded a performance improvement for our bulk-ops benchmark test case by roughly 3.5%. The local test uses memory.copy and memory.fill with 5000 bytes per operation. Note that performance wins for bulk-ops with smaller lengths are going to be way more significant.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.47%. Comparing base (c3dc480) to head (cbd684a).

:exclamation: Current head cbd684a differs from pull request most recent head afc3c5f

Please upload reports for the commit afc3c5f to get more accurate results.

Files Patch % Lines
crates/wasmi/src/engine/executor/instrs/memory.rs 93.75% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1075 +/- ## ========================================== - Coverage 80.47% 80.47% -0.01% ========================================== Files 269 269 Lines 24975 24974 -1 ========================================== - Hits 20099 20097 -2 - Misses 4876 4877 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Robbepop commented 3 months ago

Unfortunately, despite the triviality of the changes introduced by this PR local benchmarks indicate extreme performance regressions of 10-20% across the board. Probably the Rust/LLVM optimization heuristics for optimizing the loop+match construct in the interpreter's hot path got confused by this ... again.

Robbepop commented 3 months ago

https://github.com/wasmi-labs/wasmi/pull/1075/commits/afc3c5f82630346c9845a1cb8667fb580c684d00 helped to eliminate all locally observed performance regressions.