From e38998801e433ecc569ff6d58d1d0aa80eaff771 Mon Sep 17 00:00:00 2001
From: yjh <yjh465402634@gmail.com>
Date: Mon, 18 Sep 2023 13:53:06 +0800
Subject: [PATCH] Executor: Remove `LegacyInstanceReuse` strategy (#1486)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It seems the old strategy have been depracted more than one year.
So maybe it's time to clean up old strategy for wasm executor.


---
polkadot address: 15ouFh2SHpGbHtDPsJ6cXQfes9Cx1gEFnJJsJVqPGzBSTudr

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Koute <koute@users.noreply.github.com>
---
 .../pvf/prepare-worker/src/memory_stats.rs    |   2 +-
 substrate/client/cli/src/arg_enums.rs         |   8 -
 substrate/client/executor/benches/bench.rs    |   7 -
 .../runtime_blob/data_segments_snapshot.rs    |  87 ----------
 .../src/runtime_blob/globals_snapshot.rs      | 112 -------------
 .../executor/common/src/runtime_blob/mod.rs   |   4 -
 .../common/src/runtime_blob/runtime_blob.rs   |  19 +--
 .../executor/common/src/wasm_runtime.rs       |  10 --
 .../executor/src/integration_tests/linux.rs   |  84 ----------
 .../src/integration_tests/linux/smaps.rs      |  82 ----------
 .../executor/src/integration_tests/mod.rs     |  11 --
 .../client/executor/wasmtime/src/host.rs      |   2 +-
 .../executor/wasmtime/src/instance_wrapper.rs | 104 +-----------
 .../client/executor/wasmtime/src/runtime.rs   | 149 +-----------------
 .../client/executor/wasmtime/src/tests.rs     |  19 +--
 .../client/executor/wasmtime/src/util.rs      |  30 +---
 16 files changed, 20 insertions(+), 710 deletions(-)
 delete mode 100644 substrate/client/executor/common/src/runtime_blob/data_segments_snapshot.rs
 delete mode 100644 substrate/client/executor/common/src/runtime_blob/globals_snapshot.rs
 delete mode 100644 substrate/client/executor/src/integration_tests/linux.rs
 delete mode 100644 substrate/client/executor/src/integration_tests/linux/smaps.rs

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 7904dfa9cb8..c70ff56fc84 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 40d86fd9798..67acb82c2c3 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 66a82a17522..86c769f8881 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 3fd546ce445..00000000000
--- 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 <https://www.gnu.org/licenses/>.
-
-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<u8>)>,
-}
-
-impl DataSegmentsSnapshot {
-	/// Create a snapshot from the data segments from the module.
-	pub fn take(module: &RuntimeBlob) -> error::Result<Self> {
-		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::<error::Result<Vec<_>>>()?;
-
-		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<E>(
-		&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 9ba6fc55e49..00000000000
--- 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 <https://www.gnu.org/licenses/>.
-
-use super::RuntimeBlob;
-
-/// Saved value of particular exported global.
-struct SavedValue<Global> {
-	/// 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<String>);
-
-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<Global>(Vec<SavedValue<Global>>);
-
-impl<Global> GlobalsSnapshot<Global> {
-	/// 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<Instance>(
-		mutable_globals: &ExposedMutableGlobalsSet,
-		instance: &mut Instance,
-	) -> Self
-	where
-		Instance: InstanceGlobals<Global = Global>,
-	{
-		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<Instance>(&self, instance: &mut Instance)
-	where
-		Instance: InstanceGlobals<Global = Global>,
-	{
-		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 07a0945cc2b..8261d07eda5 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 24dc7e393a4..becf9e219b0 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<DataSegment> {
-		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<Item = &str> {
-		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 5dac77e59fa..d8e142b9d55 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<Option<Value>, 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 68ac37e9011..00000000000
--- 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 <https://www.gnu.org/licenses/>.
-
-//! 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 1ac570dd8d5..00000000000
--- 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 <https://www.gnu.org/licenses/>.
-
-//! 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<usize>, BTreeMap<String, usize>)>);
-
-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<String, usize> {
-		&self
-			.0
-			.iter()
-			.find(|(range, _)| addr >= range.start && addr < range.end)
-			.unwrap()
-			.1
-	}
-
-	pub fn get_rss(&self, addr: usize) -> Option<usize> {
-		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 37aed8eef96..0bd080c2435 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 <https://www.gnu.org/licenses/>.
 
-#[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 9bd3ca3dade..f8c78cbb660 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<FreeingBumpHeapAllocator>,
diff --git a/substrate/client/executor/wasmtime/src/instance_wrapper.rs b/substrate/client/executor/wasmtime/src/instance_wrapper.rs
index 6d319cce509..acc799061c2 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<C: AsContextMut> sc_allocator::Memory for MemoryWrapper<'_, C> {
-	fn with_access<R>(&self, run: impl FnOnce(&[u8]) -> R) -> R {
-		run(self.0.data(&self.1))
-	}
-
 	fn with_access_mut<R>(&mut self, run: impl FnOnce(&mut [u8]) -> R) -> R {
 		run(self.0.data_mut(&mut self.1))
 	}
 
+	fn with_access<R>(&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<C: AsContextMut> 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<wasmtime::Global> {
-		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<Table> {
 
 /// 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 23b069870aa..ae78137959b 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<StoreData>;
 
 enum Strategy {
-	LegacyInstanceReuse {
-		instance_wrapper: InstanceWrapper,
-		globals_snapshot: GlobalsSnapshot<wasmtime::Global>,
-		data_segments_snapshot: Arc<DataSegmentsSnapshot>,
-		heap_base: u32,
-	},
 	RecreateInstance(InstanceCreator),
 }
 
 struct InstanceCreator {
-	engine: wasmtime::Engine,
+	engine: Engine,
 	instance_pre: Arc<wasmtime::InstancePre<StoreData>>,
 }
 
@@ -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<DataSegmentsSnapshot>,
-}
-
 /// 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<wasmtime::InstancePre<StoreData>>,
 	instantiation_strategy: InternalInstantiationStrategy,
 }
@@ -130,26 +92,6 @@ pub struct WasmtimeRuntime {
 impl WasmModule for WasmtimeRuntime {
 	fn new_instance(&self) -> Result<Box<dyn WasmInstance>> {
 		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<AllocationStats>,
 	) -> Result<Vec<u8>> {
 		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<Option<Value>> {
 		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<wasmtime::Config,
 		InstantiationStrategy::Pooling => (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<wasmtime::Config,
 ///
 /// See [here][stack_height] for more details of the instrumentation
 ///
-/// [stack_height]: https://github.com/paritytech/wasm-utils/blob/d9432baf/src/stack_height/mod.rs#L1-L50
+/// [stack_height]: https://github.com/paritytech/wasm-instrument/blob/master/src/stack_limiter/mod.rs
 #[derive(Clone)]
 pub struct DeterministicStackLimit {
 	/// A number of logical "values" that can be pushed on the wasm stack. A trap will be triggered
@@ -458,13 +358,9 @@ pub enum InstantiationStrategy {
 
 	/// Recreate the instance from scratch on every instantiation. Very slow.
 	RecreateInstance,
-
-	/// Legacy instance reuse mechanism. DEPRECATED. Will be removed. Do not use.
-	LegacyInstanceReuse,
 }
 
 enum InternalInstantiationStrategy {
-	LegacyInstanceReuse(InstanceSnapshotData),
 	Builtin,
 }
 
@@ -655,22 +551,6 @@ where
 				.map_err(|e| WasmError::Other(format!("cannot create module: {:#}", e)))?;
 
 			match config.semantics.instantiation_strategy {
-				InstantiationStrategy::LegacyInstanceReuse => {
-					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 65093687822..e185754b076 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 c38d969ce9d..7af554c35e1 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;
-- 
GitLab