-
Notifications
You must be signed in to change notification settings - Fork 1
Feature-complete RR #5
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
base: main
Are you sure you want to change the base?
Conversation
Helps get some binaries to test with
Publish dev artifacts from CI
Test out the merge queue
…hooks into a separate module
Things left to complete: * Core wasm support * Improved error handling * Wasmtime CLI integration
… completed core wasm rr
cfallin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generally pretty good -- thanks a bunch for all of your hard work here!
I have a ton of nits below but I don't think there is anything fundamentally wrong with any of the design. This is more a lot of polish and getting details right.
My eyes did glaze over at several parts around macro usage, and innards of the component ABI. Definitely will need careful review by Alex and/or others when this lands for real. But this should be a start at least. Happy to discuss any of the below.
Notes
Func,TypedFunc, andComponentFuncasyncStructure
rrfeature. The only things exposed without the feature arerr::hooks, which offers convenient methods to hook recording/replaying mechanisms into the rest of the engine.As a result,
rrwithouthooksshould be able to land with no impact on existing Wasmtime whatsoever.Observed overheads
Between 4-8%