-
Notifications
You must be signed in to change notification settings - Fork 833
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
Su Engine: Decoding and encoding runtime state. #489
Conversation
bors try |
tryBuild failed |
bors try |
@@ -18,6 +18,8 @@ errno = "0.2.4" | |||
libc = "0.2.49" | |||
hex = "0.3.2" | |||
smallvec = "0.6.9" | |||
bincode = "1.1" | |||
colored = "1.8" |
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.
Should colored
be moved to a higher level project like wasmer
🤔
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.
How can we use dependencies in the top level workspace in wasmer-runtime-core
?
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.
I think some user, like runtime and runtime-core users may not have a need for colored output since the usage is not in a terminal, for these cases could this be moved to a higher level project?
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.
I think we can change this later when there's a need for high modularity. Currently I think it's okay to simulate Rust libstd's behavior on panic (output to stderr), though it might be good to add terminal type detection (need more code and not sure whether we really need that)
match *param { | ||
Location::GPR(x) => { | ||
let content = m.state.register_values[X64Register::GPR(x).to_index().0]; | ||
//assert!(content != MachineValue::Undefined); |
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.
Assertions needed or should they be removed?
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.
debug_assert!
is also a good choice if you think you don't need it but future changes could cause it to be triggered
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.
Added some comments to explain why this assertion should remain commented.
bors try |
bors r+ |
489: Su Engine: Decoding and encoding runtime state. r=losfair a=losfair This PR implements a managed runtime that is able to suspend a running WebAssembly program at arbitrary point in time, decode its machine state (registers and stack) into the corresponding WebAssembly abstract state (call frames, locals and the value stack), and encode the abstract state back into machine state for resuming execution later. Features enabled by this PR include: - Tier (compiler backend) switching at runtime. - Debugging with backtraces and local variables. - Suspending to disk. - Live migration. The name "Su" corresponds to 「溯」 in Chinese, originating from 「溯洄从之」 in The Book of Songs. Co-authored-by: losfair <zhy20000919@hotmail.com>
When the colored output was originally added in wasmerio#489 and there was a discussion then about that it should ideally be in a higher-level crate rather than in the runtime-core library crate. I agree with that, users of the library shouldn't be required to bring in the colored crate dependency and ideally also not have stdout/stderr output either, that should be controlled by the application that uses wasmer-runtime-core, not the library. Disabling stdout/stderr output would be more intrusive but I wanted to at least not have colored output and another crate dependency so this change removes the colored output and the "colored" crate.
792: Remove colored CLI output from wasmer-runtime-core lib r=syrusakbary a=repi When the colored output was originally added in #489 and there was a discussion then about that it should ideally be in a higher-level crate rather than in the runtime-core library crate. I agree with that, users of the library shouldn't be required to bring in the `colored` crate dependency and ideally also not have stdout/stderr output either, that should be controlled by the application that uses wasmer-runtime-core, not the library. Disabling stdout/stderr output would be more intrusive but I wanted to at least not have colored output and another crate dependency so this change removes the colored output and the `colored` crate. `colored` also had quite a few dependencies and, while well used, is not super actively maintained. So this change also removes 6 transitive dependencies of the `colored` crate which is great. This could potentially be a feature flag instead also, but would be a bit messier. Co-authored-by: Johan Andersson <repi@repi.se>
This PR implements a managed runtime that is able to suspend a running WebAssembly program at arbitrary point in time, decode its machine state (registers and stack) into the corresponding WebAssembly abstract state (call frames, locals and the value stack), and encode the abstract state back into machine state for resuming execution later.
Features enabled by this PR include:
The name "Su" corresponds to 「溯」 in Chinese, originating from 「溯洄从之」 in The Book of Songs.