-
Notifications
You must be signed in to change notification settings - Fork 816
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
Long living engine consumes endless memory when modules are loaded from file #4377
Comments
Turns out the static global The following debug logs diff --git a/lib/compiler/src/engine/trap/frame_info.rs b/lib/compiler/src/engine/trap/frame_info.rs
index 4222acc1ad..7d714ea746 100644
--- a/lib/compiler/src/engine/trap/frame_info.rs
+++ b/lib/compiler/src/engine/trap/frame_info.rs
@@ -239,5 +239,15 @@ pub fn register(
},
);
assert!(prev.is_none());
+
+ let frame_info_ranges = info.ranges.len();
+ let total_functions: usize = info.ranges.values().map(|mifi| mifi.functions.len()).sum();
+ let total_frame_info: usize = info
+ .ranges
+ .values()
+ .map(|mifi| mifi.frame_infos.len())
+ .sum();
+ println!("Elements of FRAME_INFO.ranges: {frame_info_ranges}, total functions: {total_functions}, total frame infos: {total_frame_info}");
+
Some(GlobalFrameInfoRegistration { key: max })
} print the output
This probably means the /// An RAII structure used to unregister a module's frame information when the
/// module is destroyed.
pub struct GlobalFrameInfoRegistration { Debugging the drop implementation of artifact
.internal_register_frame_info()
.map_err(|e| DeserializeError::CorruptedBinary(format!("{:?}", e)))?;
if let Some(frame_info) = artifact.internal_take_frame_info_registration() {
engine_inner.register_frame_info(frame_info);
} Now I wonder if this is a bug or the desired behaviour. |
@webmaster128 thank you for reporting this issue. I will look into this. |
Do you have some initial thoughts if and how this can be fixed? Do you think we need to adapt out codebase to use 1 engine per Module to work around the issue? |
Optimally, I want to fix this in time for the next release.
Tagging @theduke @Michael-F-Bryan to answer this, I don't have a full picture of the implications. |
I'm tracing your steps here, @webmaster128 ;-) I'm running into the same issue during benchmarks with a long-lived runtime engine. Thanks for reporting this and the reproduction is spot on too. It doesn't need to be deserialization from file by the way, the same happens for modules kept in memory. fn create_module(runtime_engine: &Engine) -> anyhow::Result<Module> {
let compiling_engine = Engine::default();
let wat = r#"
(module
(func (export "test"))
)"#;
let mut module = Module::new(&compiling_engine, wat.as_bytes())?;
let serialized_module = module.serialize()?;
for _ in 0..100 {
module = unsafe { Module::deserialize(runtime_engine, serialized_module.clone())? };
}
Ok(module)
}
pub fn wasm_module(c: &mut Criterion) {
let mut group = c.benchmark_group("wasm_module");
let runtime_engine = Engine::headless();
group.bench_function(BenchmarkId::new("wasm_module", "module"), |b| {
b.iter(|| create_module(&runtime_engine))
});
group.finish()
}
criterion_group!(benches, wasm_module,);
criterion_main!(benches);
It seems to me that if you're deserializing modules that are dropped like in the above example, it's a memory leak holding on to the artifacts that are not accessible any longer. |
The artifacts are dropped as expected at the right time (which I tested too along the way). The problem is that there is a bunch of memory for each deserialization that is owned by the engine instead of the artifact. So it lives as long as the engine. Once the engine is dropped, everything is nice and clean. |
Okay, then the memory should be freed when the artifact is dropped, shouldn't it? What else is it kept for? Or the memory should be owned by the artifact instead and will be dropped along with the artifact that way. |
I can confirm that caching a (Module, Engine) pair is a workaround for this issue because then the Engine does not live much longer than the artifact. We did this here: CosmWasm/cosmwasm#1982. Ideally we can revert once this is solved in Wasmer. |
I realize that is not a workaround for my scenario where I keep the module and thus engine for the entire time the system is running. It wouldn't make a difference storing the engine with the module or not, since the module is never dropped. On a positive note, in my use case deserialization is only done once at startup and there are less than 20 modules, so the memory usage shouldn't go through the roof. |
Hi, we ran into the same issue in https://github.com/anoma/namada. In our use case we're using singlepass compiler and we cache compiled This is what we observed on v4.3.5 (and earlier v4.3s) when we use the same
We tried to workaround this by instantiating a new
In the end, what worked for us was to keep a |
Describe the bug
When using a single long running headless engine in combination with many loads from a file system cache, the memory usage increases constantly. If modules are just instantiated from in-memory Module instances, this does not happen. This was observed by CosmWasm users.
The issue can be reproduced using the following adapted Wasmer example: master...webmaster128:demo-memory-growth. What this is doing is:
Steps to reproduce
cargo run --example engine-headless --release --features "cranelift"
in the repo rootExpected behavior
The memory usage of the example stays below a reasonable bound because the deserialized
Module
s are droppedActual behavior
Running the above example leads to the following memory profile:
A few notes here:
xcrun xctrace record --template 'Allocations' --launch ./target/release/examples/engine-headless
A different memory profiling approach shows the route through which the majority of allocations are created:
Additional context
OS:
Wasmer version:
This problem started since we refactored our caching solution such that we only use a single long living runtime engine instead of having to cache (Module, Engine) pairs. It might have been in Wasmer for a long time.
The text was updated successfully, but these errors were encountered: