From d0a2ddeb20a13a42efe44c69f31fe7fa535ce375 Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Thu, 24 Mar 2022 07:00:01 +0000 Subject: [PATCH 1/6] Add extra WASM heap pages when precompiling the runtime blob --- client/executor/wasmtime/src/runtime.rs | 40 ++++++------ client/executor/wasmtime/src/tests.rs | 81 ++++++++++++++++--------- 2 files changed, 74 insertions(+), 47 deletions(-) diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index acf54e04e07fd..7379c1c16ffde 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -414,12 +414,13 @@ pub struct Semantics { /// Configures wasmtime to use multiple threads for compiling. pub parallel_compilation: bool, + + /// The number of extra WASM pages which will be allocated + /// on top of what is requested by the WASM blob itself. + pub extra_heap_pages: u64, } pub struct Config { - /// The number of wasm pages to be mounted after instantiation. - pub heap_pages: u64, - /// The total amount of memory in bytes an instance can request. /// /// If specified, the runtime will be able to allocate only that much of wasm memory. @@ -534,21 +535,7 @@ where let (module, snapshot_data) = match code_supply_mode { CodeSupplyMode::Verbatim { blob } => { - let mut blob = instrument(blob, &config.semantics)?; - - // We don't actually need the memory to be imported so we can just convert any memory - // import into an export with impunity. This simplifies our code since `wasmtime` will - // now automatically take care of creating the memory for us, and it also allows us - // to potentially enable `wasmtime`'s instance pooling at a later date. (Imported - // memories are ineligible for pooling.) - blob.convert_memory_import_into_export()?; - blob.add_extra_heap_pages_to_memory_section( - config - .heap_pages - .try_into() - .map_err(|e| WasmError::Other(format!("invalid `heap_pages`: {}", e)))?, - )?; - + let blob = prepare_blob_for_compilation(blob, &config.semantics)?; let serialized_blob = blob.clone().serialize(); let module = wasmtime::Module::new(&engine, &serialized_blob) @@ -587,7 +574,7 @@ where Ok(WasmtimeRuntime { engine, instance_pre: Arc::new(instance_pre), snapshot_data, config }) } -fn instrument( +fn prepare_blob_for_compilation( mut blob: RuntimeBlob, semantics: &Semantics, ) -> std::result::Result { @@ -600,6 +587,19 @@ fn instrument( blob.expose_mutable_globals(); } + // We don't actually need the memory to be imported so we can just convert any memory + // import into an export with impunity. This simplifies our code since `wasmtime` will + // now automatically take care of creating the memory for us, and it also allows us + // to potentially enable `wasmtime`'s instance pooling at a later date. (Imported + // memories are ineligible for pooling.) + blob.convert_memory_import_into_export()?; + blob.add_extra_heap_pages_to_memory_section( + semantics + .extra_heap_pages + .try_into() + .map_err(|e| WasmError::Other(format!("invalid `extra_heap_pages`: {}", e)))?, + )?; + Ok(blob) } @@ -609,7 +609,7 @@ pub fn prepare_runtime_artifact( blob: RuntimeBlob, semantics: &Semantics, ) -> std::result::Result, WasmError> { - let blob = instrument(blob, semantics)?; + let blob = prepare_blob_for_compilation(blob, semantics)?; let engine = Engine::new(&common_config(semantics)?) .map_err(|e| WasmError::Other(format!("cannot create the engine: {}", e)))?; diff --git a/client/executor/wasmtime/src/tests.rs b/client/executor/wasmtime/src/tests.rs index a4ca0959da869..2d3acfcc8242b 100644 --- a/client/executor/wasmtime/src/tests.rs +++ b/client/executor/wasmtime/src/tests.rs @@ -28,8 +28,9 @@ struct RuntimeBuilder { fast_instance_reuse: bool, canonicalize_nans: bool, deterministic_stack: bool, - heap_pages: u64, + extra_heap_pages: u64, max_memory_size: Option, + precompile_runtime: bool, } impl RuntimeBuilder { @@ -41,8 +42,9 @@ impl RuntimeBuilder { fast_instance_reuse: false, canonicalize_nans: false, deterministic_stack: false, - heap_pages: 1024, + extra_heap_pages: 1024, max_memory_size: None, + precompile_runtime: false, } } @@ -58,6 +60,10 @@ impl RuntimeBuilder { self.deterministic_stack = deterministic_stack; } + fn precompile_runtime(&mut self, precompile_runtime: bool) { + self.precompile_runtime = precompile_runtime; + } + fn max_memory_size(&mut self, max_memory_size: Option) { self.max_memory_size = max_memory_size; } @@ -78,27 +84,31 @@ impl RuntimeBuilder { .expect("failed to create a runtime blob out of test runtime") }; - let rt = crate::create_runtime::( - blob, - crate::Config { - heap_pages: self.heap_pages, - max_memory_size: self.max_memory_size, - allow_missing_func_imports: true, - cache_path: None, - semantics: crate::Semantics { - fast_instance_reuse: self.fast_instance_reuse, - deterministic_stack_limit: match self.deterministic_stack { - true => Some(crate::DeterministicStackLimit { - logical_max: 65536, - native_stack_max: 256 * 1024 * 1024, - }), - false => None, - }, - canonicalize_nans: self.canonicalize_nans, - parallel_compilation: true, + let config = crate::Config { + max_memory_size: self.max_memory_size, + allow_missing_func_imports: true, + cache_path: None, + semantics: crate::Semantics { + fast_instance_reuse: self.fast_instance_reuse, + deterministic_stack_limit: match self.deterministic_stack { + true => Some(crate::DeterministicStackLimit { + logical_max: 65536, + native_stack_max: 256 * 1024 * 1024, + }), + false => None, }, + canonicalize_nans: self.canonicalize_nans, + parallel_compilation: true, + extra_heap_pages: self.extra_heap_pages, }, - ) + }; + + let rt = if self.precompile_runtime { + let artifact = crate::prepare_runtime_artifact(blob, &config.semantics).unwrap(); + unsafe { crate::create_runtime_from_artifact::(&artifact, config) } + } else { + crate::create_runtime::(blob, config) + } .expect("cannot create runtime"); Arc::new(rt) as Arc @@ -168,24 +178,36 @@ fn test_stack_depth_reaching() { } #[test] -fn test_max_memory_pages_imported_memory() { - test_max_memory_pages(true); +fn test_max_memory_pages_imported_memory_without_precompilation() { + test_max_memory_pages(true, false); +} + +#[test] +fn test_max_memory_pages_exported_memory_without_precompilation() { + test_max_memory_pages(false, false); +} + +#[test] +fn test_max_memory_pages_imported_memory_with_precompilation() { + test_max_memory_pages(true, true); } #[test] -fn test_max_memory_pages_exported_memory() { - test_max_memory_pages(false); +fn test_max_memory_pages_exported_memory_with_precompilation() { + test_max_memory_pages(false, true); } -fn test_max_memory_pages(import_memory: bool) { +fn test_max_memory_pages(import_memory: bool, precompile_runtime: bool) { fn try_instantiate( max_memory_size: Option, wat: String, + precompile_runtime: bool, ) -> Result<(), Box> { let runtime = { let mut builder = RuntimeBuilder::new_on_demand(); builder.use_wat(wat); builder.max_memory_size(max_memory_size); + builder.precompile_runtime(precompile_runtime); builder.build() }; let mut instance = runtime.new_instance()?; @@ -235,6 +257,7 @@ fn test_max_memory_pages(import_memory: bool) { */ memory(64511, None, import_memory) ), + precompile_runtime, ) .unwrap(); @@ -257,6 +280,7 @@ fn test_max_memory_pages(import_memory: bool) { // 1 initial, max is not specified. memory(1, None, import_memory) ), + precompile_runtime, ) .unwrap(); @@ -277,6 +301,7 @@ fn test_max_memory_pages(import_memory: bool) { // Max is 2048. memory(1, Some(2048), import_memory) ), + precompile_runtime, ) .unwrap(); @@ -309,6 +334,7 @@ fn test_max_memory_pages(import_memory: bool) { // Zero starting pages. memory(0, None, import_memory) ), + precompile_runtime, ) .unwrap(); @@ -341,6 +367,7 @@ fn test_max_memory_pages(import_memory: bool) { // Initial=1, meaning after heap pages mount the total will be already 1025. memory(1, None, import_memory) ), + precompile_runtime, ) .unwrap(); } @@ -353,7 +380,6 @@ fn test_instances_without_reuse_are_not_leaked() { let runtime = crate::create_runtime::( RuntimeBlob::uncompress_if_needed(wasm_binary_unwrap()).unwrap(), crate::Config { - heap_pages: 2048, max_memory_size: None, allow_missing_func_imports: true, cache_path: None, @@ -362,6 +388,7 @@ fn test_instances_without_reuse_are_not_leaked() { deterministic_stack_limit: None, canonicalize_nans: false, parallel_compilation: true, + extra_heap_pages: 2048, }, }, ) From 7bc2b271070c4f32b4ed6e2681c558fa1b583c1d Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Thu, 24 Mar 2022 07:26:27 +0000 Subject: [PATCH 2/6] Fix compilation --- client/executor/src/wasm_runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/executor/src/wasm_runtime.rs b/client/executor/src/wasm_runtime.rs index d996d7b490e88..952130e980874 100644 --- a/client/executor/src/wasm_runtime.rs +++ b/client/executor/src/wasm_runtime.rs @@ -317,11 +317,11 @@ where WasmExecutionMethod::Compiled => sc_executor_wasmtime::create_runtime::( blob, sc_executor_wasmtime::Config { - heap_pages, max_memory_size: None, allow_missing_func_imports, cache_path: cache_path.map(ToOwned::to_owned), semantics: sc_executor_wasmtime::Semantics { + extra_heap_pages: heap_pages, fast_instance_reuse: true, deterministic_stack_limit: None, canonicalize_nans: false, From 7cfdaf46a9046cdf9ca2c0ccb85d3780ec2ce7cb Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Thu, 24 Mar 2022 07:31:50 +0000 Subject: [PATCH 3/6] Fix rustdoc --- client/executor/wasmtime/src/runtime.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index 7379c1c16ffde..122a4c3facb74 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -424,11 +424,11 @@ pub struct Config { /// The total amount of memory in bytes an instance can request. /// /// If specified, the runtime will be able to allocate only that much of wasm memory. - /// This is the total number and therefore the [`Config::heap_pages`] is accounted for. + /// This is the total number and therefore the [`Config::extra_heap_pages`] is accounted for. /// /// That means that the initial number of pages of a linear memory plus the - /// [`Config::heap_pages`] multiplied by the wasm page size (64KiB) should be less than or - /// equal to `max_memory_size`, otherwise the instance won't be created. + /// [`Config::extra_heap_pages`] multiplied by the wasm page size (64KiB) should be less than + /// or equal to `max_memory_size`, otherwise the instance won't be created. /// /// Moreover, `memory.grow` will fail (return -1) if the sum of sizes of currently mounted /// and additional pages exceeds `max_memory_size`. From c7603acdecdff16cbc7fe19ca0db99c098e563a2 Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Thu, 24 Mar 2022 07:50:02 +0000 Subject: [PATCH 4/6] Fix rustdoc for real this time --- client/executor/wasmtime/src/runtime.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index 122a4c3facb74..cbe1359d28a6c 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -424,11 +424,12 @@ pub struct Config { /// The total amount of memory in bytes an instance can request. /// /// If specified, the runtime will be able to allocate only that much of wasm memory. - /// This is the total number and therefore the [`Config::extra_heap_pages`] is accounted for. + /// This is the total number and therefore the [`Semantics::extra_heap_pages`] is accounted + /// for. /// /// That means that the initial number of pages of a linear memory plus the - /// [`Config::extra_heap_pages`] multiplied by the wasm page size (64KiB) should be less than - /// or equal to `max_memory_size`, otherwise the instance won't be created. + /// [`Semantics::extra_heap_pages`] multiplied by the wasm page size (64KiB) should be less + /// than or equal to `max_memory_size`, otherwise the instance won't be created. /// /// Moreover, `memory.grow` will fail (return -1) if the sum of sizes of currently mounted /// and additional pages exceeds `max_memory_size`. From b30d899653144027f64cdd8ec3cdbbdc0d396844 Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Thu, 24 Mar 2022 07:54:36 +0000 Subject: [PATCH 5/6] Fix benches compilation --- client/executor/benches/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/executor/benches/bench.rs b/client/executor/benches/bench.rs index 20632536571b2..49ea8be50624e 100644 --- a/client/executor/benches/bench.rs +++ b/client/executor/benches/bench.rs @@ -55,11 +55,11 @@ fn initialize(runtime: &[u8], method: Method) -> Arc { sc_executor_wasmtime::create_runtime::( blob, sc_executor_wasmtime::Config { - heap_pages, max_memory_size: None, allow_missing_func_imports, cache_path: None, semantics: sc_executor_wasmtime::Semantics { + extra_heap_pages: heap_pages, fast_instance_reuse, deterministic_stack_limit: None, canonicalize_nans: false, From be8bfd065c5230d69d89c58195025458e08490e3 Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Thu, 24 Mar 2022 08:21:02 +0000 Subject: [PATCH 6/6] Improve the builder in `sc-executor-wasmtime`'s tests --- client/executor/wasmtime/src/tests.rs | 47 +++++++++++++-------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/client/executor/wasmtime/src/tests.rs b/client/executor/wasmtime/src/tests.rs index 2d3acfcc8242b..d5b92f2f24a76 100644 --- a/client/executor/wasmtime/src/tests.rs +++ b/client/executor/wasmtime/src/tests.rs @@ -48,33 +48,38 @@ impl RuntimeBuilder { } } - fn use_wat(&mut self, code: String) { + fn use_wat(&mut self, code: String) -> &mut Self { self.code = Some(code); + self } - fn canonicalize_nans(&mut self, canonicalize_nans: bool) { + fn canonicalize_nans(&mut self, canonicalize_nans: bool) -> &mut Self { self.canonicalize_nans = canonicalize_nans; + self } - fn deterministic_stack(&mut self, deterministic_stack: bool) { + fn deterministic_stack(&mut self, deterministic_stack: bool) -> &mut Self { self.deterministic_stack = deterministic_stack; + self } - fn precompile_runtime(&mut self, precompile_runtime: bool) { + fn precompile_runtime(&mut self, precompile_runtime: bool) -> &mut Self { self.precompile_runtime = precompile_runtime; + self } - fn max_memory_size(&mut self, max_memory_size: Option) { + fn max_memory_size(&mut self, max_memory_size: Option) -> &mut Self { self.max_memory_size = max_memory_size; + self } - fn build(self) -> Arc { + fn build(&mut self) -> Arc { let blob = { let wasm: Vec; let wasm = match self.code { None => wasm_binary_unwrap(), - Some(wat) => { + Some(ref wat) => { wasm = wat::parse_str(wat).expect("wat parsing failed"); &wasm }, @@ -117,11 +122,7 @@ impl RuntimeBuilder { #[test] fn test_nan_canonicalization() { - let runtime = { - let mut builder = RuntimeBuilder::new_on_demand(); - builder.canonicalize_nans(true); - builder.build() - }; + let runtime = RuntimeBuilder::new_on_demand().canonicalize_nans(true).build(); let mut instance = runtime.new_instance().expect("failed to instantiate a runtime"); @@ -160,12 +161,10 @@ fn test_nan_canonicalization() { fn test_stack_depth_reaching() { const TEST_GUARD_PAGE_SKIP: &str = include_str!("test-guard-page-skip.wat"); - let runtime = { - let mut builder = RuntimeBuilder::new_on_demand(); - builder.use_wat(TEST_GUARD_PAGE_SKIP.to_string()); - builder.deterministic_stack(true); - builder.build() - }; + let runtime = RuntimeBuilder::new_on_demand() + .use_wat(TEST_GUARD_PAGE_SKIP.to_string()) + .deterministic_stack(true) + .build(); let mut instance = runtime.new_instance().expect("failed to instantiate a runtime"); match instance.call_export("test-many-locals", &[]).unwrap_err() { @@ -203,13 +202,11 @@ fn test_max_memory_pages(import_memory: bool, precompile_runtime: bool) { wat: String, precompile_runtime: bool, ) -> Result<(), Box> { - let runtime = { - let mut builder = RuntimeBuilder::new_on_demand(); - builder.use_wat(wat); - builder.max_memory_size(max_memory_size); - builder.precompile_runtime(precompile_runtime); - builder.build() - }; + let runtime = RuntimeBuilder::new_on_demand() + .use_wat(wat) + .max_memory_size(max_memory_size) + .precompile_runtime(precompile_runtime) + .build(); let mut instance = runtime.new_instance()?; let _ = instance.call_export("main", &[])?; Ok(())