-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Improve Runtime execution by caching runtime lookup #276
Changes from 2 commits
bae0f5d
e97441b
45a58ca
aa88a45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,10 +17,35 @@ | |
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<HashMap<u64, RunWith>> = Mutex::new(HashMap::new()); | ||
} | ||
|
||
// 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); | ||
h.finish() | ||
} | ||
|
||
fn safe_call<F, U>(f: F) -> Result<U> | ||
where F: ::std::panic::UnwindSafe + FnOnce() -> U | ||
{ | ||
|
@@ -60,6 +85,11 @@ pub struct NativeExecutor<D: NativeExecutionDispatch + Sync + Send> { | |
impl<D: NativeExecutionDispatch + Sync + Send> NativeExecutor<D> { | ||
/// 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 +118,25 @@ impl<D: NativeExecutionDispatch + Sync + Send> CodeExecutor for NativeExecutor<D | |
method: &str, | ||
data: &[u8], | ||
) -> Result<Vec<u8>> { | ||
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(|| { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prefer less complexity and indentation at the cost of more lines, like:
|
||
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) | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<E: Externalities>( | ||
/// Call a given method in the given wasm-module runtime. | ||
pub fn call_in_wasm_module<E: Externalities>( | ||
&self, | ||
ext: &mut E, | ||
code: &[u8], | ||
module: &Module, | ||
method: &str, | ||
data: &[u8], | ||
) -> Result<Vec<u8>> { | ||
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::<E>::resolver()) | ||
)?; | ||
|
@@ -529,6 +527,21 @@ impl CodeExecutor for WasmExecutor { | |
} | ||
} | ||
|
||
impl CodeExecutor for WasmExecutor { | ||
type Error = Error; | ||
|
||
fn call<E: Externalities>( | ||
&self, | ||
ext: &mut E, | ||
code: &[u8], | ||
method: &str, | ||
data: &[u8], | ||
) -> Result<Vec<u8>> { | ||
let module = Module::from_buffer(code).expect("all modules compiled with rustc are valid wasm code; qed"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably an invalid expectation now, since the code is coming from chain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Taken from the previous code base. So we expect this to sort-of happen? What do we want to do in that case then? Have the result come back with a custom Error? How do expect to recover from invalid-wasm-on-chain (like at all?)? Can make that a custom error (or error_chain this one), but unsure if this shouldn't be part of a specific PR for that then... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can't really recover in the context of this particular block/state, but we shouldn't panic all the same. should basically just be a "invalid block" that can never be built on and we hope that the consensus algorithm eventually provides us another. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doesn't need to be done in this PR though since as you said it was already there. worth making an issue/TODO note in the codebase There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replace this case with a |
||
self.call_in_wasm_module(ext, &module, method, data) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since
XxHash
is not crypto-secure, it's worth putting a comment here about why we don't consider it possible that two different code preimages will be able to hash to the sameu64
. Something like "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".