From b647561c44aacc69ea7bb195dcbb8df377f851e3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 3 Feb 2022 09:17:04 -0600 Subject: [PATCH] memfd: Some minor follow-ups (#3759) * Tweak memfd-related features crates This commit changes the `memfd` feature for the `wasmtime-cli` crate from an always-on feature to a default-on feature which can be disabled at compile time. Additionally the `pooling-allocator` feature is also given similar treatment. Additionally some documentation was added for the `memfd` feature on the `wasmtime` crate. * Don't store `Arc` in `InstanceAllocationRequest` Instead store `&Arc` to avoid having the clone that lives in `InstanceAllocationRequest` not actually going anywhere. Otherwise all instance allocation requires an extra clone to create it for the request and an extra decrement when the request goes away. Internally clones are made as necessary when creating instances. * Enable the pooling allocator by default for `wasmtime-cli` While perhaps not the most useful option since the CLI doesn't have a great way to take advantage of this it probably makes sense to at least match the features of `wasmtime` itself. * Fix some lints and issues * More compile fixes --- Cargo.toml | 12 ++++++++++-- crates/runtime/src/instance/allocator.rs | 12 +++++------- crates/runtime/src/instance/allocator/pooling.rs | 8 ++++---- .../runtime/src/instance/allocator/pooling/uffd.rs | 2 +- crates/runtime/src/memfd.rs | 2 ++ crates/wasmtime/Cargo.toml | 6 ++++++ crates/wasmtime/src/instance.rs | 4 ++-- crates/wasmtime/src/module.rs | 4 ++-- crates/wasmtime/src/store.rs | 3 ++- crates/wasmtime/src/trampoline.rs | 3 ++- crates/wasmtime/src/trampoline/func.rs | 3 ++- 11 files changed, 38 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e618593f02..9fd56d95f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ path = "src/bin/wasmtime.rs" doc = false [dependencies] -wasmtime = { path = "crates/wasmtime", version = "0.33.0", default-features = false, features = ['cache', 'cranelift', 'pooling-allocator', 'memfd'] } +wasmtime = { path = "crates/wasmtime", version = "0.33.0", default-features = false, features = ['cache', 'cranelift'] } wasmtime-cache = { path = "crates/cache", version = "=0.33.0" } wasmtime-cranelift = { path = "crates/cranelift", version = "=0.33.0" } wasmtime-environ = { path = "crates/environ", version = "=0.33.0" } @@ -89,12 +89,20 @@ exclude = [ ] [features] -default = ["jitdump", "wasmtime/wat", "wasmtime/parallel-compilation", "wasi-nn"] +default = [ + "jitdump", + "wasmtime/wat", + "wasmtime/parallel-compilation", + "wasi-nn", + "memfd", + "pooling-allocator", +] jitdump = ["wasmtime/jitdump"] vtune = ["wasmtime/vtune"] wasi-crypto = ["wasmtime-wasi-crypto"] wasi-nn = ["wasmtime-wasi-nn"] uffd = ["wasmtime/uffd"] +memfd = ["wasmtime/memfd"] pooling-allocator = ["wasmtime/pooling-allocator"] all-arch = ["wasmtime/all-arch"] posix-signals-on-macos = ["wasmtime/posix-signals-on-macos"] diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index 4b9b61397a..320537defc 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -33,7 +33,7 @@ pub use self::pooling::{ /// Represents a request for a new runtime instance. pub struct InstanceAllocationRequest<'a> { /// The module being instantiated. - pub module: Arc, + pub module: &'a Arc, /// The unique ID of the module being allocated within this engine. pub unique_id: Option, @@ -42,7 +42,7 @@ pub struct InstanceAllocationRequest<'a> { pub image_base: usize, /// If using MemFD-based memories, the backing MemFDs. - pub memfds: Option>, + pub memfds: Option<&'a Arc>, /// Descriptors about each compiled function, such as the offset from /// `image_base`. @@ -672,7 +672,7 @@ impl OnDemandInstanceAllocator { &self, module: &Module, store: &mut StorePtr, - memfds: &Option>, + memfds: Option<&Arc>, ) -> Result, InstantiationError> { let creator = self .mem_creator @@ -686,9 +686,7 @@ impl OnDemandInstanceAllocator { let defined_memory_idx = module .defined_memory_index(memory_idx) .expect("Skipped imports, should never be None"); - let memfd_image = memfds - .as_ref() - .and_then(|memfds| memfds.get_memory_image(defined_memory_idx)); + let memfd_image = memfds.and_then(|memfds| memfds.get_memory_image(defined_memory_idx)); memories.push( Memory::new_dynamic( @@ -723,7 +721,7 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { &self, mut req: InstanceAllocationRequest, ) -> Result { - let memories = self.create_memories(&req.module, &mut req.store, &req.memfds)?; + let memories = self.create_memories(&req.module, &mut req.store, req.memfds)?; let tables = Self::create_tables(&req.module, &mut req.store)?; let host_state = std::mem::replace(&mut req.host_state, Box::new(())); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index ae860588ba..a0ab594694 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -378,7 +378,7 @@ impl InstancePool { index, instance, &self.memories, - &req.memfds, + req.memfds, self.memories.max_wasm_pages, )?; @@ -506,7 +506,7 @@ impl InstancePool { instance_idx: usize, instance: &mut Instance, memories: &MemoryPool, - maybe_memfds: &Option>, + maybe_memfds: Option<&Arc>, max_pages: u64, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -1468,7 +1468,7 @@ mod test { handles.push( instances .allocate(InstanceAllocationRequest { - module: module.clone(), + module: &module, unique_id: None, image_base: 0, functions, @@ -1494,7 +1494,7 @@ mod test { ); match instances.allocate(InstanceAllocationRequest { - module: module.clone(), + module: &module, unique_id: None, functions, image_base: 0, diff --git a/crates/runtime/src/instance/allocator/pooling/uffd.rs b/crates/runtime/src/instance/allocator/pooling/uffd.rs index be16ca2db1..787aec0397 100644 --- a/crates/runtime/src/instance/allocator/pooling/uffd.rs +++ b/crates/runtime/src/instance/allocator/pooling/uffd.rs @@ -579,7 +579,7 @@ mod test { handles.push( instances .allocate(InstanceAllocationRequest { - module: module.clone(), + module: &module, memfds: None, unique_id: None, image_base: 0, diff --git a/crates/runtime/src/memfd.rs b/crates/runtime/src/memfd.rs index 7009044ca2..5cf4cfbcb4 100644 --- a/crates/runtime/src/memfd.rs +++ b/crates/runtime/src/memfd.rs @@ -471,6 +471,7 @@ impl MemFdSlot { Ok(()) } + #[allow(dead_code)] // ignore warnings as this is only used in some cfgs pub(crate) fn clear_and_remain_ready(&mut self) -> Result<()> { assert!(self.dirty); // madvise the image range. This will throw away dirty pages, @@ -511,6 +512,7 @@ impl MemFdSlot { self.image.is_some() } + #[allow(dead_code)] // ignore warnings as this is only used in some cfgs pub(crate) fn is_dirty(&self) -> bool { self.dirty } diff --git a/crates/wasmtime/Cargo.toml b/crates/wasmtime/Cargo.toml index 00cb081dec..ebf0dd22c5 100644 --- a/crates/wasmtime/Cargo.toml +++ b/crates/wasmtime/Cargo.toml @@ -90,4 +90,10 @@ all-arch = ["wasmtime-cranelift/all-arch"] # need portable signal handling. posix-signals-on-macos = ["wasmtime-runtime/posix-signals-on-macos"] +# Enables, on Linux, the usage of memfd mappings to enable instantiation to use +# copy-on-write to initialize linear memory for wasm modules which have +# compatible linear memories. +# +# Enabling this feature has no effect on non-Linux platforms or when the `uffd` +# feature is enabled. memfd = ["wasmtime-runtime/memfd"] diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 9968762102..4161a10a54 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -706,9 +706,9 @@ impl<'a> Instantiator<'a> { .engine() .allocator() .allocate(InstanceAllocationRequest { - module: compiled_module.module().clone(), + module: compiled_module.module(), unique_id: Some(compiled_module.unique_id()), - memfds: self.cur.module.memfds().clone(), + memfds: self.cur.module.memfds(), image_base: compiled_module.code().as_ptr() as usize, functions: compiled_module.functions(), imports: self.cur.build(), diff --git a/crates/wasmtime/src/module.rs b/crates/wasmtime/src/module.rs index 09c2d3f485..1662388e03 100644 --- a/crates/wasmtime/src/module.rs +++ b/crates/wasmtime/src/module.rs @@ -723,8 +723,8 @@ impl Module { &self.inner.signatures } - pub(crate) fn memfds(&self) -> &Option> { - &self.inner.memfds + pub(crate) fn memfds(&self) -> Option<&Arc> { + self.inner.memfds.as_ref() } /// Looks up the module upvar value at the `index` specified. diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index e6b4c8709a..263a862430 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -418,6 +418,7 @@ impl Store { // part of `Func::call` to guarantee that the `callee: *mut VMContext` // is never null. let default_callee = unsafe { + let module = Arc::new(wasmtime_environ::Module::default()); OnDemandInstanceAllocator::default() .allocate(InstanceAllocationRequest { host_state: Box::new(()), @@ -425,7 +426,7 @@ impl Store { functions, shared_signatures: None.into(), imports: Default::default(), - module: Arc::new(wasmtime_environ::Module::default()), + module: &module, unique_id: None, memfds: None, store: StorePtr::empty(), diff --git a/crates/wasmtime/src/trampoline.rs b/crates/wasmtime/src/trampoline.rs index 02e0b51c81..89974d6e04 100644 --- a/crates/wasmtime/src/trampoline.rs +++ b/crates/wasmtime/src/trampoline.rs @@ -38,9 +38,10 @@ fn create_handle( // Use the on-demand allocator when creating handles associated with host objects // The configured instance allocator should only be used when creating module instances // as we don't want host objects to count towards instance limits. + let module = Arc::new(module); let handle = OnDemandInstanceAllocator::new(config.mem_creator.clone(), 0).allocate( InstanceAllocationRequest { - module: Arc::new(module), + module: &module, unique_id: None, memfds: None, functions, diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index 77d5f26d18..db8fdcc3d0 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -157,10 +157,11 @@ pub unsafe fn create_raw_function( module .exports .insert(String::new(), EntityIndex::Function(func_id)); + let module = Arc::new(module); Ok( OnDemandInstanceAllocator::default().allocate(InstanceAllocationRequest { - module: Arc::new(module), + module: &module, unique_id: None, memfds: None, functions: &functions,