diff --git a/polkadot/node/core/pvf/prepare-worker/src/memory_stats.rs b/polkadot/node/core/pvf/prepare-worker/src/memory_stats.rs index 7904dfa9cb88..c70ff56fc84d 100644 --- a/polkadot/node/core/pvf/prepare-worker/src/memory_stats.rs +++ b/polkadot/node/core/pvf/prepare-worker/src/memory_stats.rs @@ -151,7 +151,7 @@ pub mod memory_tracker { /// Module for dealing with the `ru_maxrss` (peak resident memory) stat from `getrusage`. /// /// NOTE: `getrusage` with the `RUSAGE_THREAD` parameter is only supported on Linux. `RUSAGE_SELF` -/// works on MacOS, but we need to get the max rss only for the preparation thread. Gettng it for +/// works on MacOS, but we need to get the max rss only for the preparation thread. Getting it for /// the current process would conflate the stats of previous jobs run by the process. #[cfg(target_os = "linux")] pub mod max_rss_stat { diff --git a/substrate/client/cli/src/arg_enums.rs b/substrate/client/cli/src/arg_enums.rs index 40d86fd97988..67acb82c2c30 100644 --- a/substrate/client/cli/src/arg_enums.rs +++ b/substrate/client/cli/src/arg_enums.rs @@ -38,12 +38,6 @@ pub enum WasmtimeInstantiationStrategy { /// Recreate the instance from scratch on every instantiation. Very slow. RecreateInstance, - - /// Legacy instance reuse mechanism. DEPRECATED. Will be removed in the future. - /// - /// Should only be used in case of encountering any issues with the new default - /// instantiation strategy. - LegacyInstanceReuse, } /// The default [`WasmtimeInstantiationStrategy`]. @@ -92,8 +86,6 @@ pub fn execution_method_from_cli( sc_service::config::WasmtimeInstantiationStrategy::Pooling, WasmtimeInstantiationStrategy::RecreateInstance => sc_service::config::WasmtimeInstantiationStrategy::RecreateInstance, - WasmtimeInstantiationStrategy::LegacyInstanceReuse => - sc_service::config::WasmtimeInstantiationStrategy::LegacyInstanceReuse, }, } } diff --git a/substrate/client/executor/benches/bench.rs b/substrate/client/executor/benches/bench.rs index 66a82a175221..86c769f88811 100644 --- a/substrate/client/executor/benches/bench.rs +++ b/substrate/client/executor/benches/bench.rs @@ -150,13 +150,6 @@ fn bench_call_instance(c: &mut Criterion) { let _ = env_logger::try_init(); let strategies = [ - ( - "legacy_instance_reuse", - Method::Compiled { - instantiation_strategy: InstantiationStrategy::LegacyInstanceReuse, - precompile: false, - }, - ), ( "recreate_instance_vanilla", Method::Compiled { diff --git a/substrate/client/executor/common/src/runtime_blob/data_segments_snapshot.rs b/substrate/client/executor/common/src/runtime_blob/data_segments_snapshot.rs deleted file mode 100644 index 3fd546ce4457..000000000000 --- a/substrate/client/executor/common/src/runtime_blob/data_segments_snapshot.rs +++ /dev/null @@ -1,87 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -use super::RuntimeBlob; -use crate::error::{self, Error}; -use std::mem; -use wasm_instrument::parity_wasm::elements::Instruction; - -/// This is a snapshot of data segments specialzied for a particular instantiation. -/// -/// Note that this assumes that no mutable globals are used. -#[derive(Clone)] -pub struct DataSegmentsSnapshot { - /// The list of data segments represented by (offset, contents). - data_segments: Vec<(u32, Vec)>, -} - -impl DataSegmentsSnapshot { - /// Create a snapshot from the data segments from the module. - pub fn take(module: &RuntimeBlob) -> error::Result { - let data_segments = module - .data_segments() - .into_iter() - .map(|mut segment| { - // Just replace contents of the segment since the segments will be discarded later - // anyway. - let contents = mem::take(segment.value_mut()); - - let init_expr = match segment.offset() { - Some(offset) => offset.code(), - // Return if the segment is passive - None => return Err(Error::SharedMemUnsupported), - }; - - // [op, End] - if init_expr.len() != 2 { - return Err(Error::InitializerHasTooManyExpressions) - } - let offset = match &init_expr[0] { - Instruction::I32Const(v) => *v as u32, - Instruction::GetGlobal(_) => { - // In a valid wasm file, initializer expressions can only refer imported - // globals. - // - // At the moment of writing the Substrate Runtime Interface does not provide - // any globals. There is nothing that prevents us from supporting this - // if/when we gain those. - return Err(Error::ImportedGlobalsUnsupported) - }, - insn => return Err(Error::InvalidInitializerExpression(format!("{:?}", insn))), - }; - - Ok((offset, contents)) - }) - .collect::>>()?; - - Ok(Self { data_segments }) - } - - /// Apply the given snapshot to a linear memory. - /// - /// Linear memory interface is represented by a closure `memory_set`. - pub fn apply( - &self, - mut memory_set: impl FnMut(u32, &[u8]) -> Result<(), E>, - ) -> Result<(), E> { - for (offset, contents) in &self.data_segments { - memory_set(*offset, contents)?; - } - Ok(()) - } -} diff --git a/substrate/client/executor/common/src/runtime_blob/globals_snapshot.rs b/substrate/client/executor/common/src/runtime_blob/globals_snapshot.rs deleted file mode 100644 index 9ba6fc55e49c..000000000000 --- a/substrate/client/executor/common/src/runtime_blob/globals_snapshot.rs +++ /dev/null @@ -1,112 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -use super::RuntimeBlob; - -/// Saved value of particular exported global. -struct SavedValue { - /// The handle of this global which can be used to refer to this global. - handle: Global, - /// The global value that was observed during the snapshot creation. - value: sp_wasm_interface::Value, -} - -/// An adapter for a wasm module instance that is focused on getting and setting globals. -pub trait InstanceGlobals { - /// A handle to a global which can be used to get or set a global variable. This is supposed to - /// be a lightweight handle, like an index or an Rc-like smart-pointer, which is cheap to clone. - type Global: Clone; - /// Get a handle to a global by it's export name. - /// - /// The requested export is must exist in the exported list, and it should be a mutable global. - fn get_global(&mut self, export_name: &str) -> Self::Global; - /// Get the current value of the global. - fn get_global_value(&mut self, global: &Self::Global) -> sp_wasm_interface::Value; - /// Update the current value of the global. - /// - /// The global behind the handle is guaranteed to be mutable and the value to be the same type - /// as the global. - fn set_global_value(&mut self, global: &Self::Global, value: sp_wasm_interface::Value); -} - -/// A set of exposed mutable globals. -/// -/// This is set of globals required to create a [`GlobalsSnapshot`] and that are collected from -/// a runtime blob that was instrumented by -/// [`RuntimeBlob::expose_mutable_globals`](super::RuntimeBlob::expose_mutable_globals`). - -/// If the code wasn't instrumented then it would be empty and snapshot would do nothing. -pub struct ExposedMutableGlobalsSet(Vec); - -impl ExposedMutableGlobalsSet { - /// Collect the set from the given runtime blob. See the struct documentation for details. - pub fn collect(runtime_blob: &RuntimeBlob) -> Self { - let global_names = - runtime_blob.exported_internal_global_names().map(ToOwned::to_owned).collect(); - Self(global_names) - } -} - -/// A snapshot of a global variables values. This snapshot can be later used for restoring the -/// values to the preserved state. -/// -/// Technically, a snapshot stores only values of mutable global variables. This is because -/// immutable global variables always have the same values. -/// -/// We take it from an instance rather from a module because the start function could potentially -/// change any of the mutable global values. -pub struct GlobalsSnapshot(Vec>); - -impl GlobalsSnapshot { - /// Take a snapshot of global variables for a given instance. - /// - /// # Panics - /// - /// This function panics if the instance doesn't correspond to the module from which the - /// [`ExposedMutableGlobalsSet`] was collected. - pub fn take( - mutable_globals: &ExposedMutableGlobalsSet, - instance: &mut Instance, - ) -> Self - where - Instance: InstanceGlobals, - { - let global_names = &mutable_globals.0; - let mut saved_values = Vec::with_capacity(global_names.len()); - - for global_name in global_names { - let handle = instance.get_global(global_name); - let value = instance.get_global_value(&handle); - saved_values.push(SavedValue { handle, value }); - } - - Self(saved_values) - } - - /// Apply the snapshot to the given instance. - /// - /// This instance must be the same that was used for creation of this snapshot. - pub fn apply(&self, instance: &mut Instance) - where - Instance: InstanceGlobals, - { - for saved_value in &self.0 { - instance.set_global_value(&saved_value.handle, saved_value.value); - } - } -} diff --git a/substrate/client/executor/common/src/runtime_blob/mod.rs b/substrate/client/executor/common/src/runtime_blob/mod.rs index 07a0945cc2b6..8261d07eda5e 100644 --- a/substrate/client/executor/common/src/runtime_blob/mod.rs +++ b/substrate/client/executor/common/src/runtime_blob/mod.rs @@ -46,10 +46,6 @@ //! is free of any floating point operations, which is a useful step towards making instances //! produced from such a module deterministic. -mod data_segments_snapshot; -mod globals_snapshot; mod runtime_blob; -pub use data_segments_snapshot::DataSegmentsSnapshot; -pub use globals_snapshot::{ExposedMutableGlobalsSet, GlobalsSnapshot, InstanceGlobals}; pub use runtime_blob::RuntimeBlob; diff --git a/substrate/client/executor/common/src/runtime_blob/runtime_blob.rs b/substrate/client/executor/common/src/runtime_blob/runtime_blob.rs index 24dc7e393a4b..becf9e219b0b 100644 --- a/substrate/client/executor/common/src/runtime_blob/runtime_blob.rs +++ b/substrate/client/executor/common/src/runtime_blob/runtime_blob.rs @@ -20,8 +20,8 @@ use crate::{error::WasmError, wasm_runtime::HeapAllocStrategy}; use wasm_instrument::{ export_mutable_globals, parity_wasm::elements::{ - deserialize_buffer, serialize, DataSegment, ExportEntry, External, Internal, MemorySection, - MemoryType, Module, Section, + deserialize_buffer, serialize, ExportEntry, External, Internal, MemorySection, MemoryType, + Module, Section, }, }; @@ -52,11 +52,6 @@ impl RuntimeBlob { Ok(Self { raw_module }) } - /// Extract the data segments from the given wasm code. - pub(super) fn data_segments(&self) -> Vec { - self.raw_module.data_section().map(|ds| ds.entries()).unwrap_or(&[]).to_vec() - } - /// The number of globals defined in locally in this module. pub fn declared_globals_count(&self) -> u32 { self.raw_module @@ -190,16 +185,6 @@ impl RuntimeBlob { Ok(()) } - /// Returns an iterator of all globals which were exported by [`expose_mutable_globals`]. - pub(super) fn exported_internal_global_names(&self) -> impl Iterator { - let exports = self.raw_module.export_section().map(|es| es.entries()).unwrap_or(&[]); - exports.iter().filter_map(|export| match export.internal() { - Internal::Global(_) if export.field().starts_with("exported_internal_global") => - Some(export.field()), - _ => None, - }) - } - /// Scans the wasm blob for the first section with the name that matches the given. Returns the /// contents of the custom section if found or `None` otherwise. pub fn custom_section_contents(&self, section_name: &str) -> Option<&[u8]> { diff --git a/substrate/client/executor/common/src/wasm_runtime.rs b/substrate/client/executor/common/src/wasm_runtime.rs index 5dac77e59fa7..d8e142b9d559 100644 --- a/substrate/client/executor/common/src/wasm_runtime.rs +++ b/substrate/client/executor/common/src/wasm_runtime.rs @@ -115,16 +115,6 @@ pub trait WasmInstance: Send { /// /// This method is only suitable for getting immutable globals. fn get_global_const(&mut self, name: &str) -> Result, Error>; - - /// **Testing Only**. This function returns the base address of the linear memory. - /// - /// This is meant to be the starting address of the memory mapped area for the linear memory. - /// - /// This function is intended only for a specific test that measures physical memory - /// consumption. - fn linear_memory_base_ptr(&self) -> Option<*const u8> { - None - } } /// Defines the heap pages allocation strategy the wasm runtime should use. diff --git a/substrate/client/executor/src/integration_tests/linux.rs b/substrate/client/executor/src/integration_tests/linux.rs deleted file mode 100644 index 68ac37e9011a..000000000000 --- a/substrate/client/executor/src/integration_tests/linux.rs +++ /dev/null @@ -1,84 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Tests that are only relevant for Linux. - -mod smaps; - -use super::mk_test_runtime; -use crate::WasmExecutionMethod; -use codec::Encode as _; -use sc_executor_common::wasm_runtime::DEFAULT_HEAP_ALLOC_STRATEGY; - -use self::smaps::Smaps; - -#[test] -fn memory_consumption_compiled() { - let _ = sp_tracing::try_init_simple(); - - if std::env::var("RUN_TEST").is_ok() { - memory_consumption(WasmExecutionMethod::Compiled { - instantiation_strategy: - sc_executor_wasmtime::InstantiationStrategy::LegacyInstanceReuse, - }); - } else { - // We need to run the test in isolation, to not getting interfered by the other tests. - let executable = std::env::current_exe().unwrap(); - let status = std::process::Command::new(executable) - .env("RUN_TEST", "1") - .args(&["--nocapture", "memory_consumption_compiled"]) - .status() - .unwrap(); - - assert!(status.success()); - } -} - -fn memory_consumption(wasm_method: WasmExecutionMethod) { - // This aims to see if linear memory stays backed by the physical memory after a runtime call. - // - // For that we make a series of runtime calls, probing the RSS for the VMA matching the linear - // memory. After the call we expect RSS to be equal to 0. - - let runtime = mk_test_runtime(wasm_method, DEFAULT_HEAP_ALLOC_STRATEGY); - - let mut instance = runtime.new_instance().unwrap(); - let heap_base = instance - .get_global_const("__heap_base") - .expect("`__heap_base` is valid") - .expect("`__heap_base` exists") - .as_i32() - .expect("`__heap_base` is an `i32`"); - - fn probe_rss(instance: &dyn sc_executor_common::wasm_runtime::WasmInstance) -> usize { - let base_addr = instance.linear_memory_base_ptr().unwrap() as usize; - Smaps::new().get_rss(base_addr).expect("failed to get rss") - } - - instance - .call_export("test_dirty_plenty_memory", &(heap_base as u32, 1u32).encode()) - .unwrap(); - let probe_1 = probe_rss(&*instance); - instance - .call_export("test_dirty_plenty_memory", &(heap_base as u32, 1024u32).encode()) - .unwrap(); - let probe_2 = probe_rss(&*instance); - - assert_eq!(probe_1, 0); - assert_eq!(probe_2, 0); -} diff --git a/substrate/client/executor/src/integration_tests/linux/smaps.rs b/substrate/client/executor/src/integration_tests/linux/smaps.rs deleted file mode 100644 index 1ac570dd8d5f..000000000000 --- a/substrate/client/executor/src/integration_tests/linux/smaps.rs +++ /dev/null @@ -1,82 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! A tool for extracting information about the memory consumption of the current process from -//! the procfs. - -use std::{collections::BTreeMap, ops::Range}; - -/// An interface to the /proc/self/smaps -/// -/// See docs about [procfs on kernel.org][procfs] -/// -/// [procfs]: https://www.kernel.org/doc/html/latest/filesystems/proc.html -pub struct Smaps(Vec<(Range, BTreeMap)>); - -impl Smaps { - pub fn new() -> Self { - let regex_start = regex::RegexBuilder::new("^([0-9a-f]+)-([0-9a-f]+)") - .multi_line(true) - .build() - .unwrap(); - let regex_kv = regex::RegexBuilder::new(r#"^([^:]+):\s*(\d+) kB"#) - .multi_line(true) - .build() - .unwrap(); - let smaps = std::fs::read_to_string("/proc/self/smaps").unwrap(); - let boundaries: Vec<_> = regex_start - .find_iter(&smaps) - .map(|matched| matched.start()) - .chain(std::iter::once(smaps.len())) - .collect(); - - let mut output = Vec::new(); - for window in boundaries.windows(2) { - let chunk = &smaps[window[0]..window[1]]; - let caps = regex_start.captures(chunk).unwrap(); - let start = usize::from_str_radix(caps.get(1).unwrap().as_str(), 16).unwrap(); - let end = usize::from_str_radix(caps.get(2).unwrap().as_str(), 16).unwrap(); - - let values = regex_kv - .captures_iter(chunk) - .map(|cap| { - let key = cap.get(1).unwrap().as_str().to_owned(); - let value = cap.get(2).unwrap().as_str().parse().unwrap(); - (key, value) - }) - .collect(); - - output.push((start..end, values)); - } - - Self(output) - } - - fn get_map(&self, addr: usize) -> &BTreeMap { - &self - .0 - .iter() - .find(|(range, _)| addr >= range.start && addr < range.end) - .unwrap() - .1 - } - - pub fn get_rss(&self, addr: usize) -> Option { - self.get_map(addr).get("Rss").cloned() - } -} diff --git a/substrate/client/executor/src/integration_tests/mod.rs b/substrate/client/executor/src/integration_tests/mod.rs index 37aed8eef96a..0bd080c24357 100644 --- a/substrate/client/executor/src/integration_tests/mod.rs +++ b/substrate/client/executor/src/integration_tests/mod.rs @@ -16,9 +16,6 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -#[cfg(target_os = "linux")] -mod linux; - use assert_matches::assert_matches; use codec::{Decode, Encode}; use sc_executor_common::{ @@ -81,14 +78,6 @@ macro_rules! test_wasm_execution { instantiation_strategy: sc_executor_wasmtime::InstantiationStrategy::Pooling }); } - - #[test] - fn [<$method_name _compiled_legacy_instance_reuse>]() { - let _ = sp_tracing::try_init_simple(); - $method_name(WasmExecutionMethod::Compiled { - instantiation_strategy: sc_executor_wasmtime::InstantiationStrategy::LegacyInstanceReuse - }); - } } }; } diff --git a/substrate/client/executor/wasmtime/src/host.rs b/substrate/client/executor/wasmtime/src/host.rs index 9bd3ca3dade5..f8c78cbb660e 100644 --- a/substrate/client/executor/wasmtime/src/host.rs +++ b/substrate/client/executor/wasmtime/src/host.rs @@ -32,7 +32,7 @@ use crate::{instance_wrapper::MemoryWrapper, runtime::StoreData, util}; pub struct HostState { /// The allocator instance to keep track of allocated memory. /// - /// This is stored as an `Option` as we need to temporarly set this to `None` when we are + /// This is stored as an `Option` as we need to temporarily set this to `None` when we are /// allocating/deallocating memory. The problem being that we can only mutable access `caller` /// once. allocator: Option, diff --git a/substrate/client/executor/wasmtime/src/instance_wrapper.rs b/substrate/client/executor/wasmtime/src/instance_wrapper.rs index 6d319cce509e..acc799061c27 100644 --- a/substrate/client/executor/wasmtime/src/instance_wrapper.rs +++ b/substrate/client/executor/wasmtime/src/instance_wrapper.rs @@ -116,14 +116,14 @@ impl EntryPoint { pub(crate) struct MemoryWrapper<'a, C>(pub &'a wasmtime::Memory, pub &'a mut C); impl sc_allocator::Memory for MemoryWrapper<'_, C> { - fn with_access(&self, run: impl FnOnce(&[u8]) -> R) -> R { - run(self.0.data(&self.1)) - } - fn with_access_mut(&mut self, run: impl FnOnce(&mut [u8]) -> R) -> R { run(self.0.data_mut(&mut self.1)) } + fn with_access(&self, run: impl FnOnce(&[u8]) -> R) -> R { + run(self.0.data(&self.1)) + } + fn grow(&mut self, additional: u32) -> std::result::Result<(), ()> { self.0 .grow(&mut self.1, additional as u64) @@ -153,11 +153,6 @@ impl sc_allocator::Memory for MemoryWrapper<'_, C> { /// routines. pub struct InstanceWrapper { instance: Instance, - /// The memory instance of the `instance`. - /// - /// It is important to make sure that we don't make any copies of this to make it easier to - /// proof - memory: Memory, store: Store, } @@ -177,7 +172,7 @@ impl InstanceWrapper { store.data_mut().memory = Some(memory); store.data_mut().table = table; - Ok(InstanceWrapper { instance, memory, store }) + Ok(InstanceWrapper { instance, store }) } /// Resolves a substrate entrypoint by the given name. @@ -280,11 +275,6 @@ impl InstanceWrapper { _ => Err("Unknown value type".into()), } } - - /// Get a global with the given `name`. - pub fn get_global(&mut self, name: &str) -> Option { - self.instance.get_global(&mut self.store, name) - } } /// Extract linear memory instance from the given instance. @@ -311,76 +301,6 @@ fn get_table(instance: &Instance, ctx: &mut Store) -> Option { /// Functions related to memory. impl InstanceWrapper { - /// Returns the pointer to the first byte of the linear memory for this instance. - pub fn base_ptr(&self) -> *const u8 { - self.memory.data_ptr(&self.store) - } - - /// If possible removes physical backing from the allocated linear memory which - /// leads to returning the memory back to the system; this also zeroes the memory - /// as a side-effect. - pub fn decommit(&mut self) { - if self.memory.data_size(&self.store) == 0 { - return - } - - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - use std::sync::Once; - - unsafe { - let ptr = self.memory.data_ptr(&self.store); - let len = self.memory.data_size(&self.store); - - // Linux handles MADV_DONTNEED reliably. The result is that the given area - // is unmapped and will be zeroed on the next pagefault. - if libc::madvise(ptr as _, len, libc::MADV_DONTNEED) != 0 { - static LOGGED: Once = Once::new(); - LOGGED.call_once(|| { - log::warn!( - "madvise(MADV_DONTNEED) failed: {}", - std::io::Error::last_os_error(), - ); - }); - } else { - return; - } - } - } else if #[cfg(target_os = "macos")] { - use std::sync::Once; - - unsafe { - let ptr = self.memory.data_ptr(&self.store); - let len = self.memory.data_size(&self.store); - - // On MacOS we can simply overwrite memory mapping. - if libc::mmap( - ptr as _, - len, - libc::PROT_READ | libc::PROT_WRITE, - libc::MAP_FIXED | libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, - -1, - 0, - ) == libc::MAP_FAILED { - static LOGGED: Once = Once::new(); - LOGGED.call_once(|| { - log::warn!( - "Failed to decommit WASM instance memory through mmap: {}", - std::io::Error::last_os_error(), - ); - }); - } else { - return; - } - } - } - } - - // If we're on an unsupported OS or the memory couldn't have been - // decommited for some reason then just manually zero it out. - self.memory.data_mut(self.store.as_context_mut()).fill(0); - } - pub(crate) fn store(&self) -> &Store { &self.store } @@ -389,17 +309,3 @@ impl InstanceWrapper { &mut self.store } } - -#[test] -fn decommit_works() { - let engine = wasmtime::Engine::default(); - let code = wat::parse_str("(module (memory (export \"memory\") 1 4))").unwrap(); - let module = wasmtime::Module::new(&engine, code).unwrap(); - let linker = wasmtime::Linker::new(&engine); - let instance_pre = linker.instantiate_pre(&module).unwrap(); - let mut wrapper = InstanceWrapper::new(&engine, &instance_pre).unwrap(); - unsafe { *wrapper.memory.data_ptr(&wrapper.store) = 42 }; - assert_eq!(unsafe { *wrapper.memory.data_ptr(&wrapper.store) }, 42); - wrapper.decommit(); - assert_eq!(unsafe { *wrapper.memory.data_ptr(&wrapper.store) }, 0); -} diff --git a/substrate/client/executor/wasmtime/src/runtime.rs b/substrate/client/executor/wasmtime/src/runtime.rs index 23b069870aa3..ae78137959be 100644 --- a/substrate/client/executor/wasmtime/src/runtime.rs +++ b/substrate/client/executor/wasmtime/src/runtime.rs @@ -27,9 +27,7 @@ use crate::{ use sc_allocator::{AllocationStats, FreeingBumpHeapAllocator}; use sc_executor_common::{ error::{Error, Result, WasmError}, - runtime_blob::{ - self, DataSegmentsSnapshot, ExposedMutableGlobalsSet, GlobalsSnapshot, RuntimeBlob, - }, + runtime_blob::RuntimeBlob, util::checked_range, wasm_runtime::{HeapAllocStrategy, InvokeMethod, WasmInstance, WasmModule}, }; @@ -69,17 +67,11 @@ impl StoreData { pub(crate) type Store = wasmtime::Store; enum Strategy { - LegacyInstanceReuse { - instance_wrapper: InstanceWrapper, - globals_snapshot: GlobalsSnapshot, - data_segments_snapshot: Arc, - heap_base: u32, - }, RecreateInstance(InstanceCreator), } struct InstanceCreator { - engine: wasmtime::Engine, + engine: Engine, instance_pre: Arc>, } @@ -89,40 +81,10 @@ impl InstanceCreator { } } -struct InstanceGlobals<'a> { - instance: &'a mut InstanceWrapper, -} - -impl<'a> runtime_blob::InstanceGlobals for InstanceGlobals<'a> { - type Global = wasmtime::Global; - - fn get_global(&mut self, export_name: &str) -> Self::Global { - self.instance - .get_global(export_name) - .expect("get_global is guaranteed to be called with an export name of a global; qed") - } - - fn get_global_value(&mut self, global: &Self::Global) -> Value { - util::from_wasmtime_val(global.get(&mut self.instance.store_mut())) - } - - fn set_global_value(&mut self, global: &Self::Global, value: Value) { - global.set(&mut self.instance.store_mut(), util::into_wasmtime_val(value)).expect( - "the value is guaranteed to be of the same value; the global is guaranteed to be mutable; qed", - ); - } -} - -/// Data required for creating instances with the fast instance reuse strategy. -struct InstanceSnapshotData { - mutable_globals: ExposedMutableGlobalsSet, - data_segments_snapshot: Arc, -} - /// A `WasmModule` implementation using wasmtime to compile the runtime module to machine code /// and execute the compiled code. pub struct WasmtimeRuntime { - engine: wasmtime::Engine, + engine: Engine, instance_pre: Arc>, instantiation_strategy: InternalInstantiationStrategy, } @@ -130,26 +92,6 @@ pub struct WasmtimeRuntime { impl WasmModule for WasmtimeRuntime { fn new_instance(&self) -> Result> { let strategy = match self.instantiation_strategy { - InternalInstantiationStrategy::LegacyInstanceReuse(ref snapshot_data) => { - let mut instance_wrapper = InstanceWrapper::new(&self.engine, &self.instance_pre)?; - let heap_base = instance_wrapper.extract_heap_base()?; - - // This function panics if the instance was created from a runtime blob different - // from which the mutable globals were collected. Here, it is easy to see that there - // is only a single runtime blob and thus it's the same that was used for both - // creating the instance and collecting the mutable globals. - let globals_snapshot = GlobalsSnapshot::take( - &snapshot_data.mutable_globals, - &mut InstanceGlobals { instance: &mut instance_wrapper }, - ); - - Strategy::LegacyInstanceReuse { - instance_wrapper, - globals_snapshot, - data_segments_snapshot: snapshot_data.data_segments_snapshot.clone(), - heap_base, - } - }, InternalInstantiationStrategy::Builtin => Strategy::RecreateInstance(InstanceCreator { engine: self.engine.clone(), instance_pre: self.instance_pre.clone(), @@ -174,39 +116,12 @@ impl WasmtimeInstance { allocation_stats: &mut Option, ) -> Result> { match &mut self.strategy { - Strategy::LegacyInstanceReuse { - ref mut instance_wrapper, - globals_snapshot, - data_segments_snapshot, - heap_base, - } => { - let entrypoint = instance_wrapper.resolve_entrypoint(method)?; - - data_segments_snapshot.apply(|offset, contents| { - util::write_memory_from( - instance_wrapper.store_mut(), - Pointer::new(offset), - contents, - ) - })?; - globals_snapshot.apply(&mut InstanceGlobals { instance: instance_wrapper }); - let allocator = FreeingBumpHeapAllocator::new(*heap_base); - - let result = - perform_call(data, instance_wrapper, entrypoint, allocator, allocation_stats); - - // Signal to the OS that we are done with the linear memory and that it can be - // reclaimed. - instance_wrapper.decommit(); - - result - }, Strategy::RecreateInstance(ref mut instance_creator) => { let mut instance_wrapper = instance_creator.instantiate()?; let heap_base = instance_wrapper.extract_heap_base()?; let entrypoint = instance_wrapper.resolve_entrypoint(method)?; - let allocator = FreeingBumpHeapAllocator::new(heap_base); + perform_call(data, &mut instance_wrapper, entrypoint, allocator, allocation_stats) }, } @@ -226,24 +141,10 @@ impl WasmInstance for WasmtimeInstance { fn get_global_const(&mut self, name: &str) -> Result> { match &mut self.strategy { - Strategy::LegacyInstanceReuse { instance_wrapper, .. } => - instance_wrapper.get_global_val(name), Strategy::RecreateInstance(ref mut instance_creator) => instance_creator.instantiate()?.get_global_val(name), } } - - fn linear_memory_base_ptr(&self) -> Option<*const u8> { - match &self.strategy { - Strategy::RecreateInstance(_) => { - // We do not keep the wasm instance around, therefore there is no linear memory - // associated with it. - None - }, - Strategy::LegacyInstanceReuse { instance_wrapper, .. } => - Some(instance_wrapper.base_ptr()), - } - } } /// Prepare a directory structure and a config file to enable wasmtime caching. @@ -338,7 +239,6 @@ fn common_config(semantics: &Semantics) -> std::result::Result (true, false), InstantiationStrategy::RecreateInstanceCopyOnWrite => (false, true), InstantiationStrategy::RecreateInstance => (false, false), - InstantiationStrategy::LegacyInstanceReuse => (false, false), }; const WASM_PAGE_SIZE: u64 = 65536; @@ -409,7 +309,7 @@ fn common_config(semantics: &Semantics) -> std::result::Result { - let data_segments_snapshot = - DataSegmentsSnapshot::take(&blob).map_err(|e| { - WasmError::Other(format!("cannot take data segments snapshot: {}", e)) - })?; - let data_segments_snapshot = Arc::new(data_segments_snapshot); - let mutable_globals = ExposedMutableGlobalsSet::collect(&blob); - - ( - module, - InternalInstantiationStrategy::LegacyInstanceReuse(InstanceSnapshotData { - data_segments_snapshot, - mutable_globals, - }), - ) - }, InstantiationStrategy::Pooling | InstantiationStrategy::PoolingCopyOnWrite | InstantiationStrategy::RecreateInstance | @@ -679,12 +559,6 @@ where } }, CodeSupplyMode::Precompiled(compiled_artifact_path) => { - if let InstantiationStrategy::LegacyInstanceReuse = - config.semantics.instantiation_strategy - { - return Err(WasmError::Other("the legacy instance reuse instantiation strategy is incompatible with precompiled modules".into())); - } - // SAFETY: The unsafety of `deserialize_file` is covered by this function. The // responsibilities to maintain the invariants are passed to the caller. // @@ -695,12 +569,6 @@ where (module, InternalInstantiationStrategy::Builtin) }, CodeSupplyMode::PrecompiledBytes(compiled_artifact_bytes) => { - if let InstantiationStrategy::LegacyInstanceReuse = - config.semantics.instantiation_strategy - { - return Err(WasmError::Other("the legacy instance reuse instantiation strategy is incompatible with precompiled modules".into())); - } - // SAFETY: The unsafety of `deserialize` is covered by this function. The // responsibilities to maintain the invariants are passed to the caller. // @@ -730,13 +598,6 @@ fn prepare_blob_for_compilation( blob = blob.inject_stack_depth_metering(logical_max)?; } - if let InstantiationStrategy::LegacyInstanceReuse = semantics.instantiation_strategy { - // When this strategy is used this must be called after all other passes which may introduce - // new global variables, otherwise they will not be reset when we call into the runtime - // again. - 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 is also necessary diff --git a/substrate/client/executor/wasmtime/src/tests.rs b/substrate/client/executor/wasmtime/src/tests.rs index 65093687822d..e185754b0769 100644 --- a/substrate/client/executor/wasmtime/src/tests.rs +++ b/substrate/client/executor/wasmtime/src/tests.rs @@ -30,7 +30,7 @@ type HostFunctions = sp_io::SubstrateHostFunctions; #[macro_export] macro_rules! test_wasm_execution { - (@no_legacy_instance_reuse $method_name:ident) => { + ($method_name:ident) => { paste::item! { #[test] fn [<$method_name _recreate_instance_cow>]() { @@ -61,19 +61,6 @@ macro_rules! test_wasm_execution { } } }; - - ($method_name:ident) => { - test_wasm_execution!(@no_legacy_instance_reuse $method_name); - - paste::item! { - #[test] - fn [<$method_name _legacy_instance_reuse>]() { - $method_name( - InstantiationStrategy::LegacyInstanceReuse - ); - } - } - }; } struct RuntimeBuilder { @@ -330,14 +317,14 @@ fn test_max_memory_pages_exported_memory_without_precompilation( test_max_memory_pages(instantiation_strategy, false, false); } -test_wasm_execution!(@no_legacy_instance_reuse test_max_memory_pages_imported_memory_with_precompilation); +test_wasm_execution!(test_max_memory_pages_imported_memory_with_precompilation); fn test_max_memory_pages_imported_memory_with_precompilation( instantiation_strategy: InstantiationStrategy, ) { test_max_memory_pages(instantiation_strategy, true, true); } -test_wasm_execution!(@no_legacy_instance_reuse test_max_memory_pages_exported_memory_with_precompilation); +test_wasm_execution!(test_max_memory_pages_exported_memory_with_precompilation); fn test_max_memory_pages_exported_memory_with_precompilation( instantiation_strategy: InstantiationStrategy, ) { diff --git a/substrate/client/executor/wasmtime/src/util.rs b/substrate/client/executor/wasmtime/src/util.rs index c38d969ce9dc..7af554c35e1b 100644 --- a/substrate/client/executor/wasmtime/src/util.rs +++ b/substrate/client/executor/wasmtime/src/util.rs @@ -21,33 +21,9 @@ use sc_executor_common::{ error::{Error, Result}, util::checked_range, }; -use sp_wasm_interface::{Pointer, Value}; +use sp_wasm_interface::Pointer; use wasmtime::{AsContext, AsContextMut}; -/// Converts a [`wasmtime::Val`] into a substrate runtime interface [`Value`]. -/// -/// Panics if the given value doesn't have a corresponding variant in `Value`. -pub fn from_wasmtime_val(val: wasmtime::Val) -> Value { - match val { - wasmtime::Val::I32(v) => Value::I32(v), - wasmtime::Val::I64(v) => Value::I64(v), - wasmtime::Val::F32(f_bits) => Value::F32(f_bits), - wasmtime::Val::F64(f_bits) => Value::F64(f_bits), - v => panic!("Given value type is unsupported by Substrate: {:?}", v), - } -} - -/// Converts a sp_wasm_interface's [`Value`] into the corresponding variant in wasmtime's -/// [`wasmtime::Val`]. -pub fn into_wasmtime_val(value: Value) -> wasmtime::Val { - match value { - Value::I32(v) => wasmtime::Val::I32(v), - Value::I64(v) => wasmtime::Val::I64(v), - Value::F32(f_bits) => wasmtime::Val::F32(f_bits), - Value::F64(f_bits) => wasmtime::Val::F64(f_bits), - } -} - /// Read data from the instance memory into a slice. /// /// Returns an error if the read would go out of the memory bounds. @@ -140,8 +116,8 @@ pub(crate) fn replace_strategy_if_broken(strategy: &mut InstantiationStrategy) { // These strategies require a working `madvise` to be sound. InstantiationStrategy::PoolingCopyOnWrite => InstantiationStrategy::Pooling, - InstantiationStrategy::RecreateInstanceCopyOnWrite | - InstantiationStrategy::LegacyInstanceReuse => InstantiationStrategy::RecreateInstance, + InstantiationStrategy::RecreateInstanceCopyOnWrite => + InstantiationStrategy::RecreateInstance, }; use std::sync::OnceLock;