Try #2250:

wasmerio · Apr 22, 2021 · ee9d2d2 · ee9d2d2
2 parents e4fd3c0 + 7f0b121
commit ee9d2d2
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 209 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/lib/engine-jit/src/artifact.rs b/lib/engine-jit/src/artifact.rs
@@ -16,7 +16,7 @@ use wasmer_engine::{
     SerializeError,
 };
 #[cfg(feature = "compiler")]
-use wasmer_engine::{Engine, SerializableFunctionFrameInfo, Tunables};
+use wasmer_engine::{Engine, Tunables};
 use wasmer_types::entity::{BoxedSlice, PrimaryMap};
 use wasmer_types::{
     FunctionIndex, LocalFunctionIndex, MemoryIndex, OwnedDataInitializer, SignatureIndex,
@@ -104,11 +104,7 @@ impl JITArtifact {
             .collect::<Vec<_>>()
             .into_boxed_slice();
 
-        let frame_infos = compilation
-            .get_frame_info()
-            .values()
-            .map(|frame_info| SerializableFunctionFrameInfo::Processed(frame_info.clone()))
-            .collect::<PrimaryMap<LocalFunctionIndex, _>>();
+        let frame_infos = compilation.get_frame_info();
 
         let serializable_compilation = SerializableCompilation {
             function_bodies: compilation.get_function_bodies(),

diff --git a/lib/engine-jit/src/serialize.rs b/lib/engine-jit/src/serialize.rs
@@ -1,10 +1,9 @@
 use loupe::MemoryUsage;
 use serde::{Deserialize, Serialize};
 use wasmer_compiler::{
-    CompileModuleInfo, CustomSection, Dwarf, FunctionBody, JumpTableOffsets, Relocation,
-    SectionIndex,
+    CompileModuleInfo, CompiledFunctionFrameInfo, CustomSection, Dwarf, FunctionBody,
+    JumpTableOffsets, Relocation, SectionIndex,
 };
-use wasmer_engine::SerializableFunctionFrameInfo;
 use wasmer_types::entity::PrimaryMap;
 use wasmer_types::{FunctionIndex, LocalFunctionIndex, OwnedDataInitializer, SignatureIndex};
 
@@ -24,10 +23,7 @@ pub struct SerializableCompilation {
     pub function_bodies: PrimaryMap<LocalFunctionIndex, FunctionBody>,
     pub function_relocations: PrimaryMap<LocalFunctionIndex, Vec<Relocation>>,
     pub function_jt_offsets: PrimaryMap<LocalFunctionIndex, JumpTableOffsets>,
-    // This is `SerializableFunctionFrameInfo` instead of `CompiledFunctionFrameInfo`,
-    // to allow lazy frame_info deserialization, we convert it to it's lazy binary
-    // format upon serialization.
-    pub function_frame_info: PrimaryMap<LocalFunctionIndex, SerializableFunctionFrameInfo>,
+    pub function_frame_info: PrimaryMap<LocalFunctionIndex, CompiledFunctionFrameInfo>,
     pub function_call_trampolines: PrimaryMap<SignatureIndex, FunctionBody>,
     pub dynamic_function_trampolines: PrimaryMap<FunctionIndex, FunctionBody>,
     pub custom_sections: PrimaryMap<SectionIndex, CustomSection>,

diff --git a/lib/engine/Cargo.toml b/lib/engine/Cargo.toml
@@ -23,7 +23,6 @@ more-asserts = "0.2"
 thiserror = "1.0"
 serde = { version = "1.0", features = ["derive", "rc"] }
 serde_bytes = { version = "0.11" }
-bincode = "1.3"
 lazy_static = "1.4"
 loupe = "0.1"
 

diff --git a/lib/engine/src/lib.rs b/lib/engine/src/lib.rs
@@ -25,7 +25,6 @@ mod engine;
 mod error;
 mod export;
 mod resolver;
-mod serialize;
 mod trap;
 mod tunables;
 
@@ -41,7 +40,6 @@ pub use crate::resolver::{
     resolve_imports, ChainableNamedResolver, NamedResolver, NamedResolverChain, NullResolver,
     Resolver,
 };
-pub use crate::serialize::SerializableFunctionFrameInfo;
 pub use crate::trap::*;
 pub use crate::tunables::Tunables;
 

diff --git a/lib/engine/src/serialize.rs b/lib/engine/src/serialize.rs
diff --git a/lib/engine/src/trap/error.rs b/lib/engine/src/trap/error.rs
@@ -3,7 +3,6 @@ use backtrace::Backtrace;
 use std::error::Error;
 use std::fmt;
 use std::sync::Arc;
-use std::sync::RwLockReadGuard;
 use wasmer_vm::{raise_user_trap, Trap, TrapCode};
 
 /// A struct representing an aborted instruction execution, with a message
@@ -56,7 +55,7 @@ impl RuntimeError {
         let info = FRAME_INFO.read().unwrap();
         let msg = message.into();
         Self::new_with_trace(
-            info,
+            &info,
             None,
             RuntimeErrorSource::Generic(msg),
             Backtrace::new_unresolved(),
@@ -72,7 +71,7 @@ impl RuntimeError {
                     // The error is already a RuntimeError, we return it directly
                     Ok(runtime_error) => *runtime_error,
                     Err(e) => Self::new_with_trace(
-                        info,
+                        &info,
                         None,
                         RuntimeErrorSource::User(e),
                         Backtrace::new_unresolved(),
@@ -85,27 +84,18 @@ impl RuntimeError {
                 signal_trap,
                 backtrace,
             } => {
-                let info = if info.should_process_frame(pc).unwrap_or(false) {
-                    drop(info);
-                    let mut info = FRAME_INFO.write().unwrap();
-                    info.maybe_process_frame(pc).unwrap();
-                    drop(info);
-                    FRAME_INFO.read().unwrap()
-                } else {
-                    info
-                };
                 let code = info
                     .lookup_trap_info(pc)
                     .map_or(signal_trap.unwrap_or(TrapCode::StackOverflow), |info| {
                         info.trap_code
                     });
-                Self::new_with_trace(info, Some(pc), RuntimeErrorSource::Trap(code), backtrace)
+                Self::new_with_trace(&info, Some(pc), RuntimeErrorSource::Trap(code), backtrace)
             }
             // A trap triggered manually from the Wasmer runtime
             Trap::Runtime {
                 trap_code,
                 backtrace,
-            } => Self::new_with_trace(info, None, RuntimeErrorSource::Trap(trap_code), backtrace),
+            } => Self::new_with_trace(&info, None, RuntimeErrorSource::Trap(trap_code), backtrace),
         }
     }
 
@@ -115,7 +105,7 @@ impl RuntimeError {
     }
 
     fn new_with_trace(
-        info: RwLockReadGuard<GlobalFrameInfo>,
+        info: &GlobalFrameInfo,
         trap_pc: Option<usize>,
         source: RuntimeErrorSource,
         native_trace: Backtrace,
@@ -144,28 +134,6 @@ impl RuntimeError {
             })
             .collect();
 
-        // If any of the frames is not processed, we adquire the lock to
-        // modify the GlobalFrameInfo module.
-        let info = if frames
-            .iter()
-            .any(|pc| info.should_process_frame(*pc).unwrap_or(false))
-        {
-            // We drop the read lock, to get a write one.
-            // Note: this is not guaranteed because it's a RwLock:
-            // the following code may cause deadlocks.
-            // TODO: clean up this code
-            drop(info);
-            {
-                let mut info = FRAME_INFO.write().unwrap();
-                for pc in frames.iter() {
-                    info.maybe_process_frame(*pc);
-                }
-            }
-            FRAME_INFO.read().unwrap()
-        } else {
-            info
-        };
-
         // Let's construct the trace
         let wasm_trace = frames
             .into_iter()

diff --git a/lib/engine/src/trap/frame_info.rs b/lib/engine/src/trap/frame_info.rs
@@ -10,7 +10,6 @@
 //! let module: ModuleInfo = ...;
 //! FRAME_INFO.register(module, compiled_functions);
 //! ```
-use crate::serialize::SerializableFunctionFrameInfo;
 use loupe::MemoryUsage;
 use std::cmp;
 use std::collections::BTreeMap;
@@ -56,39 +55,14 @@ struct ModuleInfoFrameInfo {
     start: usize,
     functions: BTreeMap<usize, FunctionInfo>,
     module: Arc<ModuleInfo>,
-    frame_infos: PrimaryMap<LocalFunctionIndex, SerializableFunctionFrameInfo>,
+    frame_infos: PrimaryMap<LocalFunctionIndex, CompiledFunctionFrameInfo>,
 }
 
 impl ModuleInfoFrameInfo {
-    fn function_debug_info(
-        &self,
-        local_index: LocalFunctionIndex,
-    ) -> &SerializableFunctionFrameInfo {
+    fn function_debug_info(&self, local_index: LocalFunctionIndex) -> &CompiledFunctionFrameInfo {
         &self.frame_infos.get(local_index).unwrap()
     }
 
-    fn process_function_debug_info(&mut self, local_index: LocalFunctionIndex) {
-        let func = self.frame_infos.get_mut(local_index).unwrap();
-        let processed: CompiledFunctionFrameInfo = match func {
-            SerializableFunctionFrameInfo::Processed(_) => {
-                // This should be a no-op on processed info
-                return;
-            }
-            SerializableFunctionFrameInfo::Unprocessed(unprocessed) => unprocessed.deserialize(),
-        };
-        *func = SerializableFunctionFrameInfo::Processed(processed)
-    }
-
-    fn processed_function_frame_info(
-        &self,
-        local_index: LocalFunctionIndex,
-    ) -> &CompiledFunctionFrameInfo {
-        match self.function_debug_info(local_index) {
-            SerializableFunctionFrameInfo::Processed(di) => &di,
-            _ => unreachable!("frame info should already be processed"),
-        }
-    }
-
     /// Gets a function given a pc
     fn function_info(&self, pc: usize) -> Option<&FunctionInfo> {
         let (end, func) = self.functions.range(pc..).next()?;
@@ -118,9 +92,7 @@ impl GlobalFrameInfo {
         // machine instruction that corresponds to `pc`, which then allows us to
         // map that to a wasm original source location.
         let rel_pos = pc - func.start;
-        let instr_map = &module
-            .processed_function_frame_info(func.local_index)
-            .address_map;
+        let instr_map = &module.function_debug_info(func.local_index).address_map;
         let pos = match instr_map
             .instructions
             .binary_search_by_key(&rel_pos, |map| map.code_offset)
@@ -171,30 +143,13 @@ impl GlobalFrameInfo {
     pub fn lookup_trap_info(&self, pc: usize) -> Option<&TrapInformation> {
         let module = self.module_info(pc)?;
         let func = module.function_info(pc)?;
-        let traps = &module.processed_function_frame_info(func.local_index).traps;
+        let traps = &module.function_debug_info(func.local_index).traps;
         let idx = traps
             .binary_search_by_key(&((pc - func.start) as u32), |info| info.code_offset)
             .ok()?;
         Some(&traps[idx])
     }
 
-    /// Should process the frame before anything?
-    pub fn should_process_frame(&self, pc: usize) -> Option<bool> {
-        let module = self.module_info(pc)?;
-        let func = module.function_info(pc)?;
-        let extra_func_info = module.function_debug_info(func.local_index);
-        Some(extra_func_info.is_unprocessed())
-    }
-
-    /// Process the frame info in case is not yet processed
-    pub fn maybe_process_frame(&mut self, pc: usize) -> Option<()> {
-        let module = self.module_info_mut(pc)?;
-        let func = module.function_info(pc)?;
-        let func_local_index = func.local_index;
-        module.process_function_debug_info(func_local_index);
-        Some(())
-    }
-
     /// Gets a module given a pc
     fn module_info(&self, pc: usize) -> Option<&ModuleInfoFrameInfo> {
         let (end, module_info) = self.ranges.range(pc..).next()?;
@@ -243,7 +198,7 @@ pub struct FunctionExtent {
 pub fn register(
     module: Arc<ModuleInfo>,
     finished_functions: &BoxedSlice<LocalFunctionIndex, FunctionExtent>,
-    frame_infos: PrimaryMap<LocalFunctionIndex, SerializableFunctionFrameInfo>,
+    frame_infos: PrimaryMap<LocalFunctionIndex, CompiledFunctionFrameInfo>,
 ) -> Option<GlobalFrameInfoRegistration> {
     let mut min = usize::max_value();
     let mut max = 0;