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<T>` in `InstanceAllocationRequest`

Instead store `&Arc<T>` 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

@@ -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"]

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<Module>,
+    pub module: &'a Arc<Module>,
 
     /// The unique ID of the module being allocated within this engine.
     pub unique_id: Option<CompiledModuleId>,
@@ -42,7 +42,7 @@ pub struct InstanceAllocationRequest<'a> {
     pub image_base: usize,
 
     /// If using MemFD-based memories, the backing MemFDs.
-    pub memfds: Option<Arc<ModuleMemFds>>,
+    pub memfds: Option<&'a Arc<ModuleMemFds>>,
 
     /// 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<Arc<ModuleMemFds>>,
+        memfds: Option<&Arc<ModuleMemFds>>,
     ) -> Result<PrimaryMap<DefinedMemoryIndex, Memory>, 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
+            let memfd_image = memfds.and_then(|memfds| memfds.get_memory_image(defined_memory_idx));
-                .as_ref()
-                .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<InstanceHandle, InstantiationError> {
-        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(()));

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<Arc<ModuleMemFds>>,
+        maybe_memfds: Option<&Arc<ModuleMemFds>>,
         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,

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,

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
     }

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"]

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(),

crates/wasmtime/src/module.rs

@@ -723,8 +723,8 @@ impl Module {
         &self.inner.signatures
     }
 
-    pub(crate) fn memfds(&self) -> &Option<Arc<ModuleMemFds>> {
+    pub(crate) fn memfds(&self) -> Option<&Arc<ModuleMemFds>> {
-        &self.inner.memfds
+        self.inner.memfds.as_ref()
     }
 
     /// Looks up the module upvar value at the `index` specified.

crates/wasmtime/src/store.rs

@@ -418,6 +418,7 @@ impl<T> Store<T> {
         // 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<T> Store<T> {
                     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(),

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,

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,