Add WasmVm::serialize method to get serialized AOT compiled module#543
Conversation
Co-Authored-By: Rachel Green <rachgreen@google.com> Signed-off-by: Matt Leon <mattleon@google.com>
WAMR does not support serialization: https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/doc/wasm_c_api.md#unsupported-list. Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
PiotrSikora
left a comment
There was a problem hiding this comment.
LGTM, sans the remaining auto nit.
Signed-off-by: Matt Leon <mattleon@google.com>
|
Thanks for your review! |
|
|
||
| Result<Module> module = | ||
| Module::compile(*engine(), std::span((uint8_t *)bytecode.data(), bytecode.size())); | ||
| Result<Module> module(::wasmtime::Error("Unable to load Wasm module: zero-length.")); |
There was a problem hiding this comment.
Nit: I don't think you need to declare this ahead of time.
There was a problem hiding this comment.
Added a comment. This is a fallback status when both precompiled and bytecode are empty.
If precompiled is not empty, then the error returned by deserialize will be output.
If bytecode is not empty, then the fail on line 197 would not be executed, and module would contain the error from compile().
There was a problem hiding this comment.
Right. I know how it works, but the style in this function is quite inconsistent, and this doesn't help with the readability.
What I meant is that you could write it like this:
if (!precompiled.empty()) {
auto module = Module::deserialize(*engine(),
std::span((uint8_t *)precompiled.data(), precompiled.size()));
if (!module) {
fail(FailState::UnableToInitializeCode,
"Failed to load Wasm module: " + module.err().message());
return false;
}
module_.emplace(module.ok());
return true;
} else {
auto module = Module::compile(*engine(), std::span((uint8_t *)bytecode.data(), bytecode.size()));
if (!module) {
fail(FailState::UnableToInitializeCode,
"Failed to load Wasm module: " + module.err().message());
return false;
}
module_.emplace(module.ok());
return true;
}
Or if you wanted to pre-allocate module and keep the fallback, then something like this:
Result<Module> module(::wasmtime::Error("Unable to load Wasm module: zero-length."));
if (!precompiled.empty()) {
module = Module::deserialize(*engine(),
std::span((uint8_t *)precompiled.data(), precompiled.size()));
if (module) {
module_.emplace(module.ok());
return true;
}
}
if (!bytecode.empty()) {
module = Module::compile(*engine(), std::span((uint8_t *)bytecode.data(), bytecode.size()));
if (module) {
module_.emplace(module.ok());
return true;
}
}
fail(FailState::UnableToInitializeCode, "Failed to load Wasm module: " + module.err().message());
return false;
Also, note that this diverges from V8 and WAMR integrations, and silently(!) ignores an invalid precompiled module, falling back to bytecode compilation.
Signed-off-by: Matt Leon <mattleon@google.com>
This PR contains both serialization and deserialization since we had no testing surface for deserialization, as pointed out in https://github.com/proxy-wasm/proxy-wasm-cpp-host/pull/522/changes#r2921882220.
This implements serialization and deserialization for wasmtime and V8.