-
Notifications
You must be signed in to change notification settings - Fork 286
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
Implement lazy Wasm to wasmi
bytecode translation
#844
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit removes all lifetime annotations from parsing related types. This is going to be important since we require the new ModuleHeader type to be stored in the Engine for all lazily compiled Wasm functions for translation purposes.
BENCHMARKS
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #844 +/- ##
==========================================
+ Coverage 80.90% 81.39% +0.48%
==========================================
Files 257 256 -1
Lines 22665 22772 +107
==========================================
+ Hits 18338 18535 +197
+ Misses 4327 4237 -90 ☔ View full report in Codecov by Sentry. |
This fixes a problem in relink_result that CompiledFunc info is oftentimes results.len() is not available at the time is it required due to uninitialized compiled function entities. Using ModuleHeader instead fixes this issue which should improve codegen in these situations and make codegen non-order dependent.
Required in last commit. (oups)
- This divides CompiledFuncEntity for eager translation and UncompiledFuncEntity for lazy translation. - This commit does not yet dispatch on UncompiledFuncEntity during execution of call instructions. - Furthermore this commit does not yet use the new LazyFuncTranslator to actually translate Wasm functions lazily.
This allows us to properly handle failed lazy translations in call instruction executions.
Now wasmi::Error takes over responsibilities of Trap. This make it possible to remove an unnecessary Box indirection.
This makes fast path faster and fixes some problems with unfair write access.
Currently Wasm benchmark CI runs out of memory for spidermonkey lazy unchecked translation. We want to see if there are memory dependencies between the different translation benchmark runs.
This reverts commit 1dd9a1e.
The cycle existed because Engine held ModuleHeader which itself held Engine. The cycle was broken by introducing EngineWeak and make ModuleHeader hold EngineWeak instead of Engine which is just a fancy wrapper around a Weak pointer to an Engine. Therefore Engine access via ModuleHeader now may fail if the Engine does no longer exist. However, due to the fact that ModuleHeader is only accessed via its Engine, this should technically never occure.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #732.
Closes #516.
Benchmarks
Execution
Local benchmarks so far concluded that call intense work load have 10-15% regressed performance when eagerly compiling functions. This is bad since ideally we would only want to suffer performance penalties when lazily compiling functions.
It might be possible to improve this situation with performance improvements in how
CodeMap
returns compiled functions.Furthermore call intense workloads are usually pretty rare in real workloads. Compute intense workloads are not significantly affected by changes introduced with lazy compilation.
Translation
After getting lazy translation up and running we were able to gather some promising benchmarks:
Spidermonkey
lazy+validated
speed-up overeager+validated
: 2.1xlazy+unchecked
speed-up overeager+unchecked
: 8.4xlazy+unchecked
speed-up overeager+validated
: 10.2xERC-20
lazy+validated
speed-up overeager+validated
: 1.8xlazy+unchecked
speed-up overeager+unchecked
: 2.7xlazy+unchecked
speed-up overeager+validated
: 3.2xERC-1155
lazy+validated
speed-up overeager+validated
: 1.95xlazy+unchecked
speed-up overeager+unchecked
: 4.0xlazy+unchecked
speed-up overeager+validated
: 4.6xTranslation Benchmarks: Conclusion
TODOs
CompilationMode
toConfig
ModuleBuilder
to remove unnecessary lifetime annotations.wasmi::Error
from the Wasmi instruction executor instead ofTrapCode
.TranslationError
.CodeMap::get
.RwLock
is used which is an unfair lock towards writers. However, when lazily compiling a function we need write access. It is possible to fix this by introducing another state that is going to be queried by threads waiting for the function to be compiled.