Merge #2183

2183: Passive elements and data are now PrimaryMaps r=syrusakbary a=syrusakbary

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description

This PR moves some fields in the `ModuleInfo` to stop using `HashMap` and start using `PrimaryMap` as the contents are not sparse but fully dense.
The PR is incentivized by #2180 as it's partially needed to stop using HashMap when possible. It doesn't completely solve it, as the `function_names` field is still a `HashMap`, but it should be trivial to solve that in a PR that improves the deserialization time.

<!-- 
Provide details regarding the change including motivation,
links to related issues, and the context of the PR.
-->

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Syrus Akbary <me@syrusakbary.com>

lib/compiler/src/translator/environ.rs

@@ -331,6 +331,14 @@ impl<'data> ModuleEnvironment<'data> {
         Ok(())
     }
 
+    pub(crate) fn reserve_passive_elements(&mut self, num: u32) -> WasmResult<()> {
+        self.result
+            .module
+            .passive_elements
+            .reserve_exact(usize::try_from(num).unwrap());
+        Ok(())
+    }
+
     pub(crate) fn declare_table_initializers(
         &mut self,
         table_index: TableIndex,
@@ -355,16 +363,7 @@ impl<'data> ModuleEnvironment<'data> {
         elem_index: ElemIndex,
         segments: Box<[FunctionIndex]>,
     ) -> WasmResult<()> {
-        let old = self
-            .result
-            .module
-            .passive_elements
-            .insert(elem_index, segments);
-        debug_assert!(
-            old.is_none(),
-            "should never get duplicate element indices, that would be a bug in `wasmer_compiler`'s \
-             translation"
-        );
+        self.result.module.passive_elements[elem_index] = segments;
         Ok(())
     }
 
@@ -416,15 +415,7 @@ impl<'data> ModuleEnvironment<'data> {
         data_index: DataIndex,
         data: &'data [u8],
     ) -> WasmResult<()> {
-        let old = self
-            .result
-            .module
-            .passive_data
-            .insert(data_index, Arc::from(data));
-        debug_assert!(
-            old.is_none(),
-            "a module can't have duplicate indices, this would be a wasmer-compiler bug"
-        );
+        self.result.module.passive_data[data_index] = Arc::from(data);
         Ok(())
     }
 

lib/compiler/src/translator/sections.rs

@@ -332,6 +332,7 @@ pub fn parse_element_section<'data>(
     environ: &mut ModuleEnvironment,
 ) -> WasmResult<()> {
     environ.reserve_table_initializers(elements.get_count())?;
+    environ.reserve_passive_elements(elements.get_count())?;
 
     for (index, entry) in elements.into_iter().enumerate() {
         let Element { kind, items, ty } = entry?;

lib/vm/src/instance/mod.rs

@@ -31,7 +31,6 @@ use memoffset::offset_of;
 use more_asserts::assert_lt;
 use std::any::Any;
 use std::cell::{Cell, RefCell};
-use std::collections::HashMap;
 use std::convert::{TryFrom, TryInto};
 use std::ffi;
 use std::fmt;
@@ -80,13 +79,12 @@ pub(crate) struct Instance {
     function_call_trampolines: BoxedSlice<SignatureIndex, VMTrampoline>,
 
     /// Passive elements in this instantiation. As `elem.drop`s happen, these
-    /// entries get removed. A missing entry is considered equivalent to an
-    /// empty slice.
-    passive_elements: RefCell<HashMap<ElemIndex, Box<[VMCallerCheckedAnyfunc]>>>,
+    /// entries get removed.
+    passive_elements: RefCell<PrimaryMap<ElemIndex, Option<Box<[VMCallerCheckedAnyfunc]>>>>,
 
     /// Passive data segments from our module. As `data.drop`s happen, entries
     /// get removed. A missing entry is considered equivalent to an empty slice.
-    passive_data: RefCell<HashMap<DataIndex, Arc<[u8]>>>,
+    passive_data: RefCell<PrimaryMap<DataIndex, Option<Arc<[u8]>>>>,
 
     /// Hosts can store arbitrary per-instance information here.
     host_state: Box<dyn Any>,
@@ -600,8 +598,9 @@ impl Instance {
         let table = self.get_table(table_index);
         let passive_elements = self.passive_elements.borrow();
         let elem = passive_elements
-            .get(&elem_index)
-            .map_or_else(|| -> &[VMCallerCheckedAnyfunc] { &[] }, |e| &**e);
+            .get(elem_index)
+            .and_then(|e| e.as_ref().map(|e| &**e))
+            .unwrap_or(&[]);
 
         if src
             .checked_add(len)
@@ -625,7 +624,7 @@ impl Instance {
         // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-elem-drop
 
         let mut passive_elements = self.passive_elements.borrow_mut();
-        passive_elements.remove(&elem_index);
+        passive_elements[elem_index] = None;
         // Note that we don't check that we actually removed an element because
         // dropping a non-passive element is a no-op (not a trap).
     }
@@ -719,8 +718,9 @@ impl Instance {
         let memory = self.get_memory(memory_index);
         let passive_data = self.passive_data.borrow();
         let data = passive_data
-            .get(&data_index)
-            .map_or(&[][..], |data| &**data);
+            .get(data_index)
+            .and_then(|data| data.as_ref().map(|d| &**d))
+            .unwrap_or(&[][..]);
 
         if src
             .checked_add(len)
@@ -746,7 +746,7 @@ impl Instance {
     /// Drop the given data segment, truncating its length to zero.
     pub(crate) fn data_drop(&self, data_index: DataIndex) {
         let mut passive_data = self.passive_data.borrow_mut();
-        passive_data.remove(&data_index);
+        passive_data[data_index] = None;
     }
 
     /// Get a table by index regardless of whether it is locally-defined or an
@@ -823,7 +823,13 @@ impl InstanceHandle {
             .map(|m| m.vmglobal())
             .collect::<PrimaryMap<LocalGlobalIndex, _>>()
             .into_boxed_slice();
-        let passive_data = RefCell::new(module.passive_data.clone());
+        let passive_data = RefCell::new(
+            module
+                .passive_data
+                .values()
+                .map(|data| Some(data.clone()))
+                .collect(),
+        );
 
         let handle = {
             let offsets = allocator.offsets().clone();
@@ -1308,22 +1314,20 @@ fn initialize_passive_elements(instance: &Instance) {
         "should only be called once, at initialization time"
     );
 
-    passive_elements.extend(
-        instance
-            .module
-            .passive_elements
+    for (segments, passive_element) in instance
+        .module
+        .passive_elements
+        .values()
+        .zip(passive_elements.values_mut())
+    {
+        if segments.is_empty() {
+            continue;
+        }
+        *passive_element = segments
             .iter()
-            .filter(|(_, segments)| !segments.is_empty())
-            .map(|(idx, segments)| {
-                (
-                    *idx,
-                    segments
-                        .iter()
-                        .map(|s| instance.get_caller_checked_anyfunc(*s))
-                        .collect(),
-                )
-            }),
-    );
+            .map(|s| Some(instance.get_caller_checked_anyfunc(*s)))
+            .collect();
+    }
 }
 
 /// Initialize the table memory from the provided initializers.

lib/vm/src/module.rs

@@ -70,10 +70,10 @@ pub struct ModuleInfo {
     pub table_initializers: Vec<TableInitializer>,
 
     /// WebAssembly passive elements.
-    pub passive_elements: HashMap<ElemIndex, Box<[FunctionIndex]>>,
+    pub passive_elements: PrimaryMap<ElemIndex, Box<[FunctionIndex]>>,
 
     /// WebAssembly passive data segments.
-    pub passive_data: HashMap<DataIndex, Arc<[u8]>>,
+    pub passive_data: PrimaryMap<DataIndex, Arc<[u8]>>,
 
     /// WebAssembly global initializers.
     pub global_initializers: PrimaryMap<LocalGlobalIndex, GlobalInit>,
@@ -125,8 +125,8 @@ impl ModuleInfo {
             exports: IndexMap::new(),
             start_function: None,
             table_initializers: Vec::new(),
-            passive_elements: HashMap::new(),
-            passive_data: HashMap::new(),
+            passive_elements: PrimaryMap::new(),
+            passive_data: PrimaryMap::new(),
             global_initializers: PrimaryMap::new(),
             function_names: HashMap::new(),
             signatures: PrimaryMap::new(),
@@ -145,7 +145,7 @@ impl ModuleInfo {
 
     /// Get the given passive element, if it exists.
     pub fn get_passive_element(&self, index: ElemIndex) -> Option<&[FunctionIndex]> {
-        self.passive_elements.get(&index).map(|es| &**es)
+        self.passive_elements.get(index).map(|es| &**es)
     }
 
     /// Get the exported signatures of the module