Refactor (#1524)

* Compute instance exports on demand. Instead having instances eagerly compute a Vec of Externs, and bumping the refcount for each Extern, compute Externs on demand. This also enables `Instance::get_export` to avoid doing a linear search. This also means that the closure returned by `get0` and friends now holds an `InstanceHandle` to dynamically hold the instance live rather than being scoped to a lifetime. * Compute module imports and exports on demand too. And compute Extern::ty on demand too. * Add a utility function for computing an ExternType. * Add a utility function for looking up a function's signature. * Add a utility function for computing the ValType of a Global. * Rename wasmtime_environ::Export to EntityIndex. This helps differentiate it from other Export types in the tree, and describes what it is. * Fix a typo in a comment. * Simplify module imports and exports. * Make `Instance::exports` return the export names. This significantly simplifies the public API, as it's relatively common to need the names, and this avoids the need to do a zip with `Module::exports`. This also changes `ImportType` and `ExportType` to have public members instead of private members and accessors, as I find that simplifies the usage particularly in cases where there are temporary instances. * Remove `Instance::module`. This doesn't quite remove `Instance`'s `module` member, it gets a step closer. * Use a InstanceHandle utility function. * Don't consume self in the `Func::get*` methods. Instead, just create a closure containing the instance handle and the export for them to call. * Use `ExactSizeIterator` to avoid needing separate `num_*` methods. * Rename `Extern::func()` etc. to `into_func()` etc. * Revise examples to avoid using `nth`. * Add convenience methods to instance for getting specific extern types. * Use the convenience functions in more tests and examples. * Avoid cloning strings for `ImportType` and `ExportType`. * Remove more obviated clone() calls. * Simplify `Func`'s closure state. * Make wasmtime::Export's fields private. This makes them more consistent with ExportType. * Fix compilation error. * Make a lifetime parameter explicit, and use better lifetime names. Instead of 'me, use 'instance and 'module to make it clear what the lifetime is. * More lifetime cleanups.
2020-04-20 13:55:33 -07:00
parent 967827f4b5
commit 9364eb1d98
57 changed files with 788 additions and 875 deletions
--- a/crates/environ/src/cranelift.rs
+++ b/crates/environ/src/cranelift.rs
@@ -203,7 +203,7 @@ fn compile(env: CompileEnv<'_>) -> Result<ModuleCacheDataTupleType, CompileError
            let func_index = env.local.func_index(*i);
            let mut context = Context::new();
            context.func.name = get_func_name(func_index);
-            context.func.signature = env.local.signatures[env.local.functions[func_index]].clone();
+            context.func.signature = env.local.func_signature(func_index).clone();
            if env.tunables.debug_info {
                context.func.collect_debug_info();
            }
--- a/crates/environ/src/func_environ.rs
+++ b/crates/environ/src/func_environ.rs
@@ -906,8 +906,8 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
        func: &mut ir::Function,
        index: FuncIndex,
    ) -> WasmResult<ir::FuncRef> {
-        let sigidx = self.module.functions[index];
-        let signature = func.import_signature(self.module.signatures[sigidx].clone());
+        let sig = self.module.func_signature(index);
+        let signature = func.import_signature(sig.clone());
        let name = get_func_name(index);
        Ok(func.import_function(ir::ExtFuncData {
            name,
--- a/crates/environ/src/lib.rs
+++ b/crates/environ/src/lib.rs
@@ -55,7 +55,7 @@ pub use crate::func_environ::BuiltinFunctionIndex;
 #[cfg(feature = "lightbeam")]
 pub use crate::lightbeam::Lightbeam;
 pub use crate::module::{
-    Export, MemoryPlan, MemoryStyle, Module, ModuleLocal, TableElements, TablePlan, TableStyle,
+    EntityIndex, MemoryPlan, MemoryStyle, Module, ModuleLocal, TableElements, TablePlan, TableStyle,
 };
 pub use crate::module_environ::{
    translate_signature, DataInitializer, DataInitializerLocation, FunctionBodyData,
--- a/crates/environ/src/module.rs
+++ b/crates/environ/src/module.rs
@@ -30,16 +30,16 @@ pub struct TableElements {
    pub elements: Box<[FuncIndex]>,
 }

-/// An entity to export.
+/// An index of an entity.
 #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub enum Export {
-    /// Function export.
+pub enum EntityIndex {
+    /// Function index.
    Function(FuncIndex),
-    /// Table export.
+    /// Table index.
    Table(TableIndex),
-    /// Memory export.
+    /// Memory index.
    Memory(MemoryIndex),
-    /// Global export.
+    /// Global index.
    Global(GlobalIndex),
 }

@@ -150,21 +150,11 @@ pub struct Module {
    /// function.
    pub local: ModuleLocal,

-    /// Names of imported functions, as well as the index of the import that
-    /// performed this import.
-    pub imported_funcs: PrimaryMap<FuncIndex, (String, String, u32)>,
-
-    /// Names of imported tables.
-    pub imported_tables: PrimaryMap<TableIndex, (String, String, u32)>,
-
-    /// Names of imported memories.
-    pub imported_memories: PrimaryMap<MemoryIndex, (String, String, u32)>,
-
-    /// Names of imported globals.
-    pub imported_globals: PrimaryMap<GlobalIndex, (String, String, u32)>,
+    /// All import records, in the order they are declared in the module.
+    pub imports: Vec<(String, String, EntityIndex)>,

    /// Exported entities.
-    pub exports: IndexMap<String, Export>,
+    pub exports: IndexMap<String, EntityIndex>,

    /// The module "start" function, if present.
    pub start_func: Option<FuncIndex>,
@@ -226,10 +216,7 @@ impl Module {
        Self {
            id: NEXT_ID.fetch_add(1, SeqCst),
            name: None,
-            imported_funcs: PrimaryMap::new(),
-            imported_tables: PrimaryMap::new(),
-            imported_memories: PrimaryMap::new(),
-            imported_globals: PrimaryMap::new(),
+            imports: Vec::new(),
            exports: IndexMap::new(),
            start_func: None,
            table_elements: Vec::new(),
@@ -344,4 +331,9 @@ impl ModuleLocal {
    pub fn is_imported_global(&self, index: GlobalIndex) -> bool {
        index.index() < self.num_imported_globals
    }
+
+    /// Convenience method for looking up the signature of a function.
+    pub fn func_signature(&self, func_index: FuncIndex) -> &ir::Signature {
+        &self.signatures[self.functions[func_index]]
+    }
 }
--- a/crates/environ/src/module_environ.rs
+++ b/crates/environ/src/module_environ.rs
@@ -1,5 +1,5 @@
 use crate::func_environ::FuncEnvironment;
-use crate::module::{Export, MemoryPlan, Module, TableElements, TablePlan};
+use crate::module::{EntityIndex, MemoryPlan, Module, TableElements, TablePlan};
 use crate::tunables::Tunables;
 use cranelift_codegen::ir;
 use cranelift_codegen::ir::{AbiParam, ArgumentPurpose};
@@ -8,7 +8,7 @@ use cranelift_entity::PrimaryMap;
 use cranelift_wasm::{
    self, translate_module, DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex,
    Memory, MemoryIndex, ModuleTranslationState, SignatureIndex, Table, TableIndex,
-    TargetEnvironment, WasmResult,
+    TargetEnvironment, WasmError, WasmResult,
 };
 use std::convert::TryFrom;
 use std::sync::Arc;
@@ -57,7 +57,6 @@ impl<'data> ModuleTranslation<'data> {
 pub struct ModuleEnvironment<'data> {
    /// The result to be filled in.
    result: ModuleTranslation<'data>,
-    imports: u32,
 }

 impl<'data> ModuleEnvironment<'data> {
@@ -72,7 +71,6 @@ impl<'data> ModuleEnvironment<'data> {
                tunables: tunables.clone(),
                module_translation: None,
            },
-            imports: 0,
        }
    }

@@ -89,7 +87,7 @@ impl<'data> ModuleEnvironment<'data> {
        Ok(self.result)
    }

-    fn declare_export(&mut self, export: Export, name: &str) -> WasmResult<()> {
+    fn declare_export(&mut self, export: EntityIndex, name: &str) -> WasmResult<()> {
        self.result
            .module
            .exports
@@ -123,6 +121,14 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
        Ok(())
    }

+    fn reserve_imports(&mut self, num: u32) -> WasmResult<()> {
+        Ok(self
+            .result
+            .module
+            .imports
+            .reserve_exact(usize::try_from(num).unwrap()))
+    }
+
    fn declare_func_import(
        &mut self,
        sig_index: SignatureIndex,
@@ -131,37 +137,33 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
    ) -> WasmResult<()> {
        debug_assert_eq!(
            self.result.module.local.functions.len(),
-            self.result.module.imported_funcs.len(),
+            self.result.module.local.num_imported_funcs,
            "Imported functions must be declared first"
        );
-        self.result.module.local.functions.push(sig_index);
-
-        self.result.module.imported_funcs.push((
-            String::from(module),
-            String::from(field),
-            self.imports,
+        let func_index = self.result.module.local.functions.push(sig_index);
+        self.result.module.imports.push((
+            module.to_owned(),
+            field.to_owned(),
+            EntityIndex::Function(func_index),
        ));
        self.result.module.local.num_imported_funcs += 1;
-        self.imports += 1;
        Ok(())
    }

    fn declare_table_import(&mut self, table: Table, module: &str, field: &str) -> WasmResult<()> {
        debug_assert_eq!(
            self.result.module.local.table_plans.len(),
-            self.result.module.imported_tables.len(),
+            self.result.module.local.num_imported_tables,
            "Imported tables must be declared first"
        );
        let plan = TablePlan::for_table(table, &self.result.tunables);
-        self.result.module.local.table_plans.push(plan);
-
-        self.result.module.imported_tables.push((
-            String::from(module),
-            String::from(field),
-            self.imports,
+        let table_index = self.result.module.local.table_plans.push(plan);
+        self.result.module.imports.push((
+            module.to_owned(),
+            field.to_owned(),
+            EntityIndex::Table(table_index),
        ));
        self.result.module.local.num_imported_tables += 1;
-        self.imports += 1;
        Ok(())
    }

@@ -173,19 +175,20 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
    ) -> WasmResult<()> {
        debug_assert_eq!(
            self.result.module.local.memory_plans.len(),
-            self.result.module.imported_memories.len(),
+            self.result.module.local.num_imported_memories,
            "Imported memories must be declared first"
        );
+        if memory.shared {
+            return Err(WasmError::Unsupported("shared memories".to_owned()));
+        }
        let plan = MemoryPlan::for_memory(memory, &self.result.tunables);
-        self.result.module.local.memory_plans.push(plan);
-
-        self.result.module.imported_memories.push((
-            String::from(module),
-            String::from(field),
-            self.imports,
+        let memory_index = self.result.module.local.memory_plans.push(plan);
+        self.result.module.imports.push((
+            module.to_owned(),
+            field.to_owned(),
+            EntityIndex::Memory(memory_index),
        ));
        self.result.module.local.num_imported_memories += 1;
-        self.imports += 1;
        Ok(())
    }

@@ -197,26 +200,16 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
    ) -> WasmResult<()> {
        debug_assert_eq!(
            self.result.module.local.globals.len(),
-            self.result.module.imported_globals.len(),
+            self.result.module.local.num_imported_globals,
            "Imported globals must be declared first"
        );
-        self.result.module.local.globals.push(global);
-
-        self.result.module.imported_globals.push((
-            String::from(module),
-            String::from(field),
-            self.imports,
+        let global_index = self.result.module.local.globals.push(global);
+        self.result.module.imports.push((
+            module.to_owned(),
+            field.to_owned(),
+            EntityIndex::Global(global_index),
        ));
        self.result.module.local.num_imported_globals += 1;
-        self.imports += 1;
-        Ok(())
-    }
-
-    fn finish_imports(&mut self) -> WasmResult<()> {
-        self.result.module.imported_funcs.shrink_to_fit();
-        self.result.module.imported_tables.shrink_to_fit();
-        self.result.module.imported_memories.shrink_to_fit();
-        self.result.module.imported_globals.shrink_to_fit();
        Ok(())
    }

@@ -262,6 +255,9 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
    }

    fn declare_memory(&mut self, memory: Memory) -> WasmResult<()> {
+        if memory.shared {
+            return Err(WasmError::Unsupported("shared memories".to_owned()));
+        }
        let plan = MemoryPlan::for_memory(memory, &self.result.tunables);
        self.result.module.local.memory_plans.push(plan);
        Ok(())
@@ -290,19 +286,19 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
    }

    fn declare_func_export(&mut self, func_index: FuncIndex, name: &str) -> WasmResult<()> {
-        self.declare_export(Export::Function(func_index), name)
+        self.declare_export(EntityIndex::Function(func_index), name)
    }

    fn declare_table_export(&mut self, table_index: TableIndex, name: &str) -> WasmResult<()> {
-        self.declare_export(Export::Table(table_index), name)
+        self.declare_export(EntityIndex::Table(table_index), name)
    }

    fn declare_memory_export(&mut self, memory_index: MemoryIndex, name: &str) -> WasmResult<()> {
-        self.declare_export(Export::Memory(memory_index), name)
+        self.declare_export(EntityIndex::Memory(memory_index), name)
    }

    fn declare_global_export(&mut self, global_index: GlobalIndex, name: &str) -> WasmResult<()> {
-        self.declare_export(Export::Global(global_index), name)
+        self.declare_export(EntityIndex::Global(global_index), name)
    }

    fn declare_start_func(&mut self, func_index: FuncIndex) -> WasmResult<()> {
@@ -421,6 +417,27 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
            .insert(func_index, name.to_string());
        Ok(())
    }
+
+    fn custom_section(&mut self, name: &'data str, _data: &'data [u8]) -> WasmResult<()> {
+        match name {
+            "webidl-bindings" | "wasm-interface-types" => Err(WasmError::Unsupported(
+                "\
+Support for interface types has temporarily been removed from `wasmtime`.
+
+For more information about this temoprary you can read on the issue online:
+
+    https://github.com/bytecodealliance/wasmtime/issues/1271
+
+and for re-adding support for interface types you can see this issue:
+
+    https://github.com/bytecodealliance/wasmtime/issues/677
+"
+                .to_owned(),
+            )),
+            // skip other sections
+            _ => Ok(()),
+        }
+    }
 }

 /// Add environment-specific function parameters.