From 6aa28e114eb0e057c8bc3ab7b017a7e9a9dfb592 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 31 Jan 2020 07:44:47 +0100 Subject: [PATCH 1/6] Rename Storage type T -> S --- lib/vm/src/context.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/vm/src/context.rs b/lib/vm/src/context.rs index 50b82dd19f..92c49b9030 100644 --- a/lib/vm/src/context.rs +++ b/lib/vm/src/context.rs @@ -74,50 +74,50 @@ pub fn do_human_address(api: A, ctx: &mut Ctx, canonical_ptr: u32, human /** context data **/ -struct ContextData { - data: Option, +struct ContextData { + data: Option, } -pub fn setup_context() -> (*mut c_void, fn(*mut c_void)) { +pub fn setup_context() -> (*mut c_void, fn(*mut c_void)) { ( - create_unmanaged_storage::(), - destroy_unmanaged_storage::, + create_unmanaged_storage::(), + destroy_unmanaged_storage::, ) } -fn create_unmanaged_storage() -> *mut c_void { - let data = ContextData:: { data: None }; +fn create_unmanaged_storage() -> *mut c_void { + let data = ContextData:: { data: None }; let state = Box::new(data); Box::into_raw(state) as *mut c_void } -unsafe fn get_data(ptr: *mut c_void) -> Box> { - Box::from_raw(ptr as *mut ContextData) +unsafe fn get_data(ptr: *mut c_void) -> Box> { + Box::from_raw(ptr as *mut ContextData) } -fn destroy_unmanaged_storage(ptr: *mut c_void) { +fn destroy_unmanaged_storage(ptr: *mut c_void) { if !ptr.is_null() { // auto-dropped with scope - let _ = unsafe { get_data::(ptr) }; + let _ = unsafe { get_data::(ptr) }; } } -pub fn with_storage_from_context(ctx: &Ctx, mut func: F) { - let mut storage: Option = take_storage(ctx); +pub fn with_storage_from_context(ctx: &Ctx, mut func: F) { + let mut storage: Option = take_storage(ctx); if let Some(data) = &mut storage { func(data); } leave_storage(ctx, storage); } -pub fn take_storage(ctx: &Ctx) -> Option { +pub fn take_storage(ctx: &Ctx) -> Option { let mut b = unsafe { get_data(ctx.data) }; let res = b.data.take(); mem::forget(b); // we do this to avoid cleanup res } -pub fn leave_storage(ctx: &Ctx, storage: Option) { +pub fn leave_storage(ctx: &Ctx, storage: Option) { let mut b = unsafe { get_data(ctx.data) }; // clean-up if needed let _ = b.data.take(); From 983e7d6a3c0f9fe0f9c435326b8def54bcd268dc Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 31 Jan 2020 08:12:46 +0100 Subject: [PATCH 2/6] Rename instance -> wasmer_instance --- lib/vm/src/instance.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index d99b6f5fab..8e5cea4de0 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -20,7 +20,7 @@ use crate::errors::{ResolveErr, Result, RuntimeErr, WasmerErr}; use crate::memory::{read_region, write_region}; pub struct Instance { - instance: wasmer_runtime_core::instance::Instance, + wasmer_instance: wasmer_runtime_core::instance::Instance, pub api: A, storage: PhantomData, } @@ -70,9 +70,8 @@ where }), }, }; - let instance = module.instantiate(&import_obj).context(WasmerErr {})?; let res = Instance { - instance, + wasmer_instance: module.instantiate(&import_obj).context(WasmerErr {})?, api, storage: PhantomData:: {}, }; @@ -81,27 +80,27 @@ where } pub fn get_gas(&self) -> u64 { - get_gas(&self.instance) + get_gas(&self.wasmer_instance) } pub fn set_gas(&mut self, gas: u64) { - set_gas(&mut self.instance, gas) + set_gas(&mut self.wasmer_instance, gas) } pub fn with_storage(&self, func: F) { - with_storage_from_context(self.instance.context(), func) + with_storage_from_context(self.wasmer_instance.context(), func) } pub fn take_storage(&self) -> Option { - take_storage(self.instance.context()) + take_storage(self.wasmer_instance.context()) } pub fn leave_storage(&self, storage: Option) { - leave_storage(self.instance.context(), storage); + leave_storage(self.wasmer_instance.context(), storage); } pub fn memory(&self, ptr: u32) -> Vec { - read_region(self.instance.context(), ptr) + read_region(self.wasmer_instance.context(), ptr) } // allocate memory in the instance and copies the given data in @@ -109,7 +108,7 @@ where pub fn allocate(&mut self, data: &[u8]) -> Result { let alloc: Func = self.func("allocate")?; let ptr = alloc.call(data.len() as u32).context(RuntimeErr {})?; - write_region(self.instance.context(), ptr, data)?; + write_region(self.wasmer_instance.context(), ptr, data)?; Ok(ptr) } @@ -127,7 +126,7 @@ where Args: WasmTypeList, Rets: WasmTypeList, { - self.instance.func(name).context(ResolveErr {}) + self.wasmer_instance.func(name).context(ResolveErr {}) } } From 9c2b7b334018b98d7575668fa51283e9e04d711b Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 31 Jan 2020 08:36:08 +0100 Subject: [PATCH 3/6] Store wasmer_runtime_core::Instance in cache --- lib/vm/src/cache.rs | 18 +++++++++++------- lib/vm/src/instance.rs | 17 ++++++++++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/vm/src/cache.rs b/lib/vm/src/cache.rs index 8691c689ec..7fe33e4c62 100644 --- a/lib/vm/src/cache.rs +++ b/lib/vm/src/cache.rs @@ -1,4 +1,5 @@ use std::fs::create_dir_all; +use std::marker::PhantomData; use std::path::PathBuf; use lru::LruCache; @@ -19,7 +20,10 @@ static MODULES_DIR: &str = "modules"; pub struct CosmCache { wasm_path: PathBuf, modules: FileSystemCache, - instances: Option>>, + instances: Option>, + // Those two don't store data but only fix type information + type_storage: PhantomData, + type_api: PhantomData, } impl CosmCache @@ -48,6 +52,8 @@ where modules, wasm_path, instances, + type_storage: PhantomData:: {}, + type_api: PhantomData:: {}, }) } @@ -78,10 +84,8 @@ where // pop from lru cache if present if let Some(cache) = &mut self.instances { - let val = cache.pop(&hash); - if let Some(inst) = val { - inst.leave_storage(Some(deps.storage)); - return Ok(inst); + if let Some(cached_instance) = cache.pop(&hash) { + return Ok(Instance::from_wasmer(cached_instance, deps)); } } @@ -100,8 +104,8 @@ where if let Some(cache) = &mut self.instances { let hash = WasmHash::generate(&id); let storage = instance.take_storage(); - let api = instance.api; // copy it - cache.put(hash, instance); + let (wasmer_instance, api) = Instance::recycle(instance); + cache.put(hash, wasmer_instance); if let Some(storage) = storage { return Some(Extern { storage, api }); } diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index 8e5cea4de0..5cd433d6d2 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -70,13 +70,24 @@ where }), }, }; + let wasmer_instance = module.instantiate(&import_obj).context(WasmerErr {})?; + Ok(Instance::from_wasmer(wasmer_instance, deps)) + } + + pub fn from_wasmer(wasmer_instance: wasmer_runtime_core::Instance, deps: Extern) -> Self { let res = Instance { - wasmer_instance: module.instantiate(&import_obj).context(WasmerErr {})?, - api, + wasmer_instance: wasmer_instance, + api: deps.api, storage: PhantomData:: {}, }; res.leave_storage(Some(deps.storage)); - Ok(res) + res + } + + /// Takes ownership of instance and decomposes it into its components. + /// The components we want to preserve are returned, the rest is dropped. + pub fn recycle(instance: Self) -> (wasmer_runtime_core::Instance, A) { + (instance.wasmer_instance, instance.api) } pub fn get_gas(&self) -> u64 { From 7ffaaefdfecc72598c604987aad54adaa3b02fbc Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 31 Jan 2020 08:37:49 +0100 Subject: [PATCH 4/6] Help understanding PhantomData in Instance --- lib/vm/src/instance.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index 5cd433d6d2..f64356dfe3 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -22,7 +22,8 @@ use crate::memory::{read_region, write_region}; pub struct Instance { wasmer_instance: wasmer_runtime_core::instance::Instance, pub api: A, - storage: PhantomData, + // This does not store data but only fixes type information + type_storage: PhantomData, } impl Instance @@ -78,7 +79,7 @@ where let res = Instance { wasmer_instance: wasmer_instance, api: deps.api, - storage: PhantomData:: {}, + type_storage: PhantomData:: {}, }; res.leave_storage(Some(deps.storage)); res From 083c34b0c7529317749ac6cb851cc54ab1e21ca9 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 31 Jan 2020 08:56:15 +0100 Subject: [PATCH 5/6] Implement cache stats --- lib/vm/src/cache.rs | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/lib/vm/src/cache.rs b/lib/vm/src/cache.rs index 7fe33e4c62..d50555ab03 100644 --- a/lib/vm/src/cache.rs +++ b/lib/vm/src/cache.rs @@ -17,10 +17,17 @@ use crate::wasm_store::{load, save, wasm_hash}; static WASM_DIR: &str = "wasm"; static MODULES_DIR: &str = "modules"; +struct Stats { + hits_instance: u32, + hits_module: u32, + misses: u32, +} + pub struct CosmCache { wasm_path: PathBuf, modules: FileSystemCache, instances: Option>, + stats: Stats, // Those two don't store data but only fix type information type_storage: PhantomData, type_api: PhantomData, @@ -52,6 +59,11 @@ where modules, wasm_path, instances, + stats: Stats { + hits_instance: 0, + hits_module: 0, + misses: 0, + }, type_storage: PhantomData:: {}, type_api: PhantomData:: {}, }) @@ -85,6 +97,7 @@ where // pop from lru cache if present if let Some(cache) = &mut self.instances { if let Some(cached_instance) = cache.pop(&hash) { + self.stats.hits_instance += 1; return Ok(Instance::from_wasmer(cached_instance, deps)); } } @@ -92,11 +105,13 @@ where // try from the module cache let res = self.modules.load_with_backend(hash, backend()); if let Ok(module) = res { + self.stats.hits_module += 1; return Instance::from_module(&module, deps); } // fall back to wasm cache (and re-compiling) - this is for backends that don't support serialization let wasm = self.load_wasm(id)?; + self.stats.misses += 1; Instance::from_code(&wasm, deps) } @@ -152,6 +167,37 @@ mod test { } } + #[test] + fn finds_cached_module() { + let tmp_dir = TempDir::new().unwrap(); + let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() }; + let id = cache.save_wasm(CONTRACT_0_7).unwrap(); + let deps = dependencies(20); + let _instance = cache.get_instance(&id, deps).unwrap(); + assert_eq!(cache.stats.hits_instance, 0); + assert_eq!(cache.stats.hits_module, 1); + assert_eq!(cache.stats.misses, 0); + } + + #[test] + fn finds_cached_instance() { + let tmp_dir = TempDir::new().unwrap(); + let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() }; + let id = cache.save_wasm(CONTRACT_0_7).unwrap(); + let deps1 = dependencies(20); + let deps2 = dependencies(20); + let deps3 = dependencies(20); + let instance1 = cache.get_instance(&id, deps1).unwrap(); + cache.store_instance(&id, instance1); + let instance2 = cache.get_instance(&id, deps2).unwrap(); + cache.store_instance(&id, instance2); + let instance3 = cache.get_instance(&id, deps3).unwrap(); + cache.store_instance(&id, instance3); + assert_eq!(cache.stats.hits_instance, 2); + assert_eq!(cache.stats.hits_module, 1); + assert_eq!(cache.stats.misses, 0); + } + #[test] fn init_cached_contract() { let tmp_dir = TempDir::new().unwrap(); From de489a9dc286b736539d237c8a2648a8b86dc414 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 31 Jan 2020 12:09:07 +0100 Subject: [PATCH 6/6] Use some useful trail implementations for Stats --- lib/vm/src/cache.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/vm/src/cache.rs b/lib/vm/src/cache.rs index d50555ab03..1b34c35679 100644 --- a/lib/vm/src/cache.rs +++ b/lib/vm/src/cache.rs @@ -17,6 +17,7 @@ use crate::wasm_store::{load, save, wasm_hash}; static WASM_DIR: &str = "wasm"; static MODULES_DIR: &str = "modules"; +#[derive(Debug, Default, Clone)] struct Stats { hits_instance: u32, hits_module: u32, @@ -59,11 +60,7 @@ where modules, wasm_path, instances, - stats: Stats { - hits_instance: 0, - hits_module: 0, - misses: 0, - }, + stats: Stats::default(), type_storage: PhantomData:: {}, type_api: PhantomData:: {}, })