From bae0f5d314fd2ebc0d917b4d034b1b76c0febd5e Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 4 Jul 2018 09:59:55 +0200 Subject: [PATCH 1/4] Improve Runtime execution by caching runtime lookup Cache whether the native or wasm runtime should be used for a given code and if the latter, keep around the parsed wasmi::Module for faster execution. --- Cargo.lock | 3 ++ substrate/executor/Cargo.toml | 3 ++ substrate/executor/src/lib.rs | 5 ++ substrate/executor/src/native_executor.rs | 57 ++++++++++++++++++----- substrate/executor/src/wasm_executor.rs | 27 ++++++++--- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0396556fa5105..eb03db6381eca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2162,7 +2162,9 @@ dependencies = [ "ed25519 0.1.0", "error-chain 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "hex-literal 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", + "lazy_static 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.64 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.64 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2173,6 +2175,7 @@ dependencies = [ "substrate-serializer 0.1.0", "substrate-state-machine 0.1.0", "triehash 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "twox-hash 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "wabt 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "wasmi 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/substrate/executor/Cargo.toml b/substrate/executor/Cargo.toml index 9e04e1ede34b6..e127b0d36ea6e 100644 --- a/substrate/executor/Cargo.toml +++ b/substrate/executor/Cargo.toml @@ -19,6 +19,9 @@ byteorder = "1.1" rustc-hex = "1.0.0" triehash = "0.1.0" hex-literal = "0.1.0" +twox-hash = "1.1.0" +lazy_static = "1.0" +parking_lot = "*" log = "0.3" [dev-dependencies] diff --git a/substrate/executor/src/lib.rs b/substrate/executor/src/lib.rs index 2983cf9f54e33..352106a74d2eb 100644 --- a/substrate/executor/src/lib.rs +++ b/substrate/executor/src/lib.rs @@ -41,8 +41,13 @@ extern crate wasmi; extern crate byteorder; extern crate rustc_hex; extern crate triehash; +extern crate parking_lot; +extern crate twox_hash; #[macro_use] extern crate log; +#[macro_use] +extern crate lazy_static; + #[macro_use] extern crate error_chain; diff --git a/substrate/executor/src/native_executor.rs b/substrate/executor/src/native_executor.rs index 5625082604c12..18034cb8b7d1f 100644 --- a/substrate/executor/src/native_executor.rs +++ b/substrate/executor/src/native_executor.rs @@ -17,10 +17,32 @@ use error::{Error, ErrorKind, Result}; use state_machine::{CodeExecutor, Externalities}; use wasm_executor::WasmExecutor; +use wasmi::Module as WasmModule; use runtime_version::RuntimeVersion; +use std::collections::HashMap; use codec::Slicable; +use twox_hash::XxHash; +use std::hash::Hasher; +use parking_lot::Mutex; use RuntimeInfo; +// For the internal Runtime Cache +enum RunWith { + NativeRuntime, + WasmRuntime(WasmModule) +} + +lazy_static! { + static ref RUNTIMES_CACHE: Mutex> = Mutex::new(HashMap::new()); +} + +// helper function to generate low-over-head caching_keys +fn gen_cache_key(code: &[u8]) -> u64 { + let mut h = XxHash::with_seed(0); + h.write(code); + h.finish() +} + fn safe_call(f: F) -> Result where F: ::std::panic::UnwindSafe + FnOnce() -> U { @@ -60,6 +82,11 @@ pub struct NativeExecutor { impl NativeExecutor { /// Create new instance. pub fn new() -> Self { + // FIXME: set this entry at compile time + RUNTIMES_CACHE.lock().insert( + gen_cache_key(D::native_equivalent()), + RunWith::NativeRuntime); + NativeExecutor { _dummy: Default::default(), } @@ -88,17 +115,25 @@ impl CodeExecutor for NativeExecutor Result> { - if code == D::native_equivalent() { - // call native - D::dispatch(ext, method, data) - } else { - let version = WasmExecutor.call(ext, code, "version", &[])?; - let version = RuntimeVersion::decode(&mut version.as_slice()); - if version.map_or(false, |v| D::VERSION.can_call_with(&v)) { - return D::dispatch(ext, method, data) - } - // call into wasm. - WasmExecutor.call(ext, code, method, data) + match RUNTIMES_CACHE.lock().entry(gen_cache_key(code)).or_insert_with(|| { + let wasm_module = WasmModule::from_buffer(code) + .expect("all modules compiled with rustc are valid wasm code; qed"); + + match WasmExecutor.call_in_wasm_module(ext, &wasm_module, "version", &[]) { + Ok(version) => { + let version = RuntimeVersion::decode(&mut version.as_slice()); + if version.map_or(false, |v| D::VERSION.can_call_with(&v)) { + RunWith::NativeRuntime + } else { + RunWith::WasmRuntime(wasm_module) + } + }, + // broken or old runtime, run with native + _ => RunWith::NativeRuntime + } + }) { + RunWith::NativeRuntime => D::dispatch(ext, method, data), + RunWith::WasmRuntime(module) => WasmExecutor.call_in_wasm_module(ext, &module, method, data) } } } diff --git a/substrate/executor/src/wasm_executor.rs b/substrate/executor/src/wasm_executor.rs index f66b81bedf918..b4084376fb2de 100644 --- a/substrate/executor/src/wasm_executor.rs +++ b/substrate/executor/src/wasm_executor.rs @@ -468,21 +468,19 @@ impl_function_executor!(this: FunctionExecutor<'e, E>, #[derive(Debug, Default, Clone)] pub struct WasmExecutor; -impl CodeExecutor for WasmExecutor { - type Error = Error; +impl WasmExecutor { - fn call( + /// Call a given method in the given wasm-module runtime. + pub fn call_in_wasm_module( &self, ext: &mut E, - code: &[u8], + module: &Module, method: &str, data: &[u8], ) -> Result> { - let module = Module::from_buffer(code).expect("all modules compiled with rustc are valid wasm code; qed"); - // start module instantiation. Don't run 'start' function yet. let intermediate_instance = ModuleInstance::new( - &module, + module, &ImportsBuilder::new() .with_resolver("env", FunctionExecutor::::resolver()) )?; @@ -529,6 +527,21 @@ impl CodeExecutor for WasmExecutor { } } +impl CodeExecutor for WasmExecutor { + type Error = Error; + + fn call( + &self, + ext: &mut E, + code: &[u8], + method: &str, + data: &[u8], + ) -> Result> { + let module = Module::from_buffer(code).expect("all modules compiled with rustc are valid wasm code; qed"); + self.call_in_wasm_module(ext, &module, method, data) + } +} + #[cfg(test)] mod tests { use super::*; From e97441b44139d1c3fc98821043e38c4177a2224d Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 4 Jul 2018 14:37:58 +0200 Subject: [PATCH 2/4] Additional comment and code style fixes --- substrate/executor/src/native_executor.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/substrate/executor/src/native_executor.rs b/substrate/executor/src/native_executor.rs index 18034cb8b7d1f..db4d571857e87 100644 --- a/substrate/executor/src/native_executor.rs +++ b/substrate/executor/src/native_executor.rs @@ -37,6 +37,9 @@ lazy_static! { } // helper function to generate low-over-head caching_keys +// it is asserted that part of the audit process that any potential on-chain code change +// will have done is to ensure that the two-x hash is different to that of any other +// :code value from the same chain fn gen_cache_key(code: &[u8]) -> u64 { let mut h = XxHash::with_seed(0); h.write(code); @@ -117,7 +120,7 @@ impl CodeExecutor for NativeExecutor Result> { match RUNTIMES_CACHE.lock().entry(gen_cache_key(code)).or_insert_with(|| { let wasm_module = WasmModule::from_buffer(code) - .expect("all modules compiled with rustc are valid wasm code; qed"); + .expect("all modules compiled with rustc are valid wasm code; qed"); match WasmExecutor.call_in_wasm_module(ext, &wasm_module, "version", &[]) { Ok(version) => { From 45a58ca3a54cbe738b632438aa359a5b3de161bc Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 4 Jul 2018 16:57:41 +0200 Subject: [PATCH 3/4] Fallback to WASM runtime if we can't call the version function --- substrate/executor/src/native_executor.rs | 33 +++++++++++------------ 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/substrate/executor/src/native_executor.rs b/substrate/executor/src/native_executor.rs index db4d571857e87..72ba2d33a727d 100644 --- a/substrate/executor/src/native_executor.rs +++ b/substrate/executor/src/native_executor.rs @@ -118,23 +118,22 @@ impl CodeExecutor for NativeExecutor Result> { - match RUNTIMES_CACHE.lock().entry(gen_cache_key(code)).or_insert_with(|| { - let wasm_module = WasmModule::from_buffer(code) - .expect("all modules compiled with rustc are valid wasm code; qed"); - - match WasmExecutor.call_in_wasm_module(ext, &wasm_module, "version", &[]) { - Ok(version) => { - let version = RuntimeVersion::decode(&mut version.as_slice()); - if version.map_or(false, |v| D::VERSION.can_call_with(&v)) { - RunWith::NativeRuntime - } else { - RunWith::WasmRuntime(wasm_module) - } - }, - // broken or old runtime, run with native - _ => RunWith::NativeRuntime - } - }) { + let mut cache = RUNTIMES_CACHE.lock(); + let r = cache.entry(gen_cache_key(code)) + .or_insert_with(|| { + let wasm_module = WasmModule::from_buffer(code) + .expect("all modules compiled with rustc are valid wasm code; qed"); + + if WasmExecutor.call_in_wasm_module(ext, &wasm_module, "version", &[]).ok() + .and_then(|version| RuntimeVersion::decode(&mut version.as_slice())) + .map_or(false, |v| D::VERSION.can_call_with(&v)) + { + RunWith::NativeRuntime + } else { + RunWith::WasmRuntime(wasm_module) + } + }); + match r { RunWith::NativeRuntime => D::dispatch(ext, method, data), RunWith::WasmRuntime(module) => WasmExecutor.call_in_wasm_module(ext, &module, method, data) } From aa88a4554686df6783bfb3ba225c587e370afa37 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 4 Jul 2018 17:23:54 +0200 Subject: [PATCH 4/4] The previous assumption that the wasm-code was compiled with rustc might be wrong now, that the code comes from the blockchain. Added Notes about that. --- substrate/executor/src/wasm_executor.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/substrate/executor/src/wasm_executor.rs b/substrate/executor/src/wasm_executor.rs index b4084376fb2de..f48a3a93bc042 100644 --- a/substrate/executor/src/wasm_executor.rs +++ b/substrate/executor/src/wasm_executor.rs @@ -490,6 +490,8 @@ impl WasmExecutor { let memory = intermediate_instance .not_started_instance() .export_by_name("memory") + // TODO: with code coming from the blockchain it isn't strictly been compiled with rustc anymore. + // these assumptions are probably not true anymore .expect("all modules compiled with rustc should have an export named 'memory'; qed") .as_memory() .expect("in module generated by rustc export named 'memory' should be a memory; qed") @@ -537,7 +539,7 @@ impl CodeExecutor for WasmExecutor { method: &str, data: &[u8], ) -> Result> { - let module = Module::from_buffer(code).expect("all modules compiled with rustc are valid wasm code; qed"); + let module = Module::from_buffer(code)?; self.call_in_wasm_module(ext, &module, method, data) } }