sc-executor: Increase maximum instance count (#1856)

Changes the maximum instances count for `wasmtime` to `64`. It also
allows to only pass in maximum `32` for `--max-runtime-instances` as
`256` was way too big. With `64` instances in total and `32` that can be
configured in maximum, there should be enough space to accommodate for
extra instances that are may required to be allocated adhoc.

---------

Co-authored-by: Koute <koute@users.noreply.github.com>

Cargo.lock

@@ -15172,6 +15172,7 @@ dependencies = [
  "libc",
  "log",
  "parity-scale-codec",
+ "parking_lot 0.12.1",
  "paste",
  "rustix 0.36.15",
  "sc-allocator",

substrate/client/cli/src/params/runtime_params.rs

@@ -22,7 +22,7 @@ use std::str::FromStr;
 /// Parameters used to config runtime.
 #[derive(Debug, Clone, Args)]
 pub struct RuntimeParams {
-	/// The size of the instances cache for each runtime. The values higher than 256 are illegal.
+	/// The size of the instances cache for each runtime. The values higher than 32 are illegal.
 	#[arg(long, default_value_t = 8, value_parser = parse_max_runtime_instances)]
 	pub max_runtime_instances: usize,
 
@@ -35,8 +35,8 @@ fn parse_max_runtime_instances(s: &str) -> Result<usize, String> {
 	let max_runtime_instances = usize::from_str(s)
 		.map_err(|_err| format!("Illegal `--max-runtime-instances` value: {s}"))?;
 
-	if max_runtime_instances > 256 {
-		Err(format!("Illegal `--max-runtime-instances` value: {max_runtime_instances} is more than the allowed maximum of `256` "))
+	if max_runtime_instances > 32 {
+		Err(format!("Illegal `--max-runtime-instances` value: {max_runtime_instances} is more than the allowed maximum of `32` "))
 	} else {
 		Ok(max_runtime_instances)
 	}

substrate/client/executor/wasmtime/Cargo.toml

@@ -16,6 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 log = "0.4.17"
 cfg-if = "1.0"
 libc = "0.2.121"
+parking_lot = "0.12.1"
 
 # When bumping wasmtime do not forget to also bump rustix
 # to exactly the same version as used by wasmtime!

substrate/client/executor/wasmtime/src/instance_wrapper.rs

@@ -19,7 +19,9 @@
 //! Defines data and logic needed for interaction with an WebAssembly instance of a substrate
 //! runtime module.
 
-use crate::runtime::{Store, StoreData};
+use std::sync::Arc;
+
+use crate::runtime::{InstanceCounter, ReleaseInstanceHandle, Store, StoreData};
 use sc_executor_common::{
 	error::{Backtrace, Error, MessageWithBacktrace, Result, WasmError},
 	wasm_runtime::InvokeMethod,
@@ -154,10 +156,19 @@ impl<C: AsContextMut> sc_allocator::Memory for MemoryWrapper<'_, C> {
 pub struct InstanceWrapper {
 	instance: Instance,
 	store: Store,
+	// NOTE: We want to decrement the instance counter *after* the store has been dropped
+	// to avoid a potential race condition, so this field must always be kept
+	// as the last field in the struct!
+	_release_instance_handle: ReleaseInstanceHandle,
 }
 
 impl InstanceWrapper {
-	pub(crate) fn new(engine: &Engine, instance_pre: &InstancePre<StoreData>) -> Result<Self> {
+	pub(crate) fn new(
+		engine: &Engine,
+		instance_pre: &InstancePre<StoreData>,
+		instance_counter: Arc<InstanceCounter>,
+	) -> Result<Self> {
+		let _release_instance_handle = instance_counter.acquire_instance();
 		let mut store = Store::new(engine, Default::default());
 		let instance = instance_pre.instantiate(&mut store).map_err(|error| {
 			WasmError::Other(format!(
@@ -172,7 +183,7 @@ impl InstanceWrapper {
 		store.data_mut().memory = Some(memory);
 		store.data_mut().table = table;
 
-		Ok(InstanceWrapper { instance, store })
+		Ok(InstanceWrapper { instance, store, _release_instance_handle })
 	}
 
 	/// Resolves a substrate entrypoint by the given name.

substrate/client/executor/wasmtime/src/runtime.rs

@@ -24,6 +24,7 @@ use crate::{
 	util::{self, replace_strategy_if_broken},
 };
 
+use parking_lot::Mutex;
 use sc_allocator::{AllocationStats, FreeingBumpHeapAllocator};
 use sc_executor_common::{
 	error::{Error, Result, WasmError},
@@ -42,6 +43,8 @@ use std::{
 };
 use wasmtime::{AsContext, Engine, Memory, Table};
 
+const MAX_INSTANCE_COUNT: u32 = 64;
+
 #[derive(Default)]
 pub(crate) struct StoreData {
 	/// This will only be set when we call into the runtime.
@@ -73,11 +76,59 @@ enum Strategy {
 struct InstanceCreator {
 	engine: Engine,
 	instance_pre: Arc<wasmtime::InstancePre<StoreData>>,
+	instance_counter: Arc<InstanceCounter>,
 }
 
 impl InstanceCreator {
 	fn instantiate(&mut self) -> Result<InstanceWrapper> {
-		InstanceWrapper::new(&self.engine, &self.instance_pre)
+		InstanceWrapper::new(&self.engine, &self.instance_pre, self.instance_counter.clone())
+	}
+}
+
+/// A handle for releasing an instance acquired by [`InstanceCounter::acquire_instance`].
+pub(crate) struct ReleaseInstanceHandle {
+	counter: Arc<InstanceCounter>,
+}
+
+impl Drop for ReleaseInstanceHandle {
+	fn drop(&mut self) {
+		{
+			let mut counter = self.counter.counter.lock();
+			*counter = counter.saturating_sub(1);
+		}
+
+		self.counter.wait_for_instance.notify_one();
+	}
+}
+
+/// Keeps track on the number of parallel instances.
+///
+/// The runtime cache keeps track on the number of parallel instances. The maximum number in the
+/// cache is less than what we have configured as [`MAX_INSTANCE_COUNT`] for wasmtime. However, the
+/// cache will create on demand instances if required. This instance counter will ensure that we are
+/// blocking when we are trying to create too many instances.
+#[derive(Default)]
+pub(crate) struct InstanceCounter {
+	counter: Mutex<u32>,
+	wait_for_instance: parking_lot::Condvar,
+}
+
+impl InstanceCounter {
+	/// Acquire an instance.
+	///
+	/// Blocks if there is no free instance available.
+	///
+	/// The returned [`ReleaseInstanceHandle`] should be dropped when the instance isn't used
+	/// anymore.
+	pub fn acquire_instance(self: Arc<Self>) -> ReleaseInstanceHandle {
+		let mut counter = self.counter.lock();
+
+		while *counter >= MAX_INSTANCE_COUNT {
+			self.wait_for_instance.wait(&mut counter);
+		}
+		*counter += 1;
+
+		ReleaseInstanceHandle { counter: self.clone() }
 	}
 }
 
@@ -87,6 +138,7 @@ pub struct WasmtimeRuntime {
 	engine: Engine,
 	instance_pre: Arc<wasmtime::InstancePre<StoreData>>,
 	instantiation_strategy: InternalInstantiationStrategy,
+	instance_counter: Arc<InstanceCounter>,
 }
 
 impl WasmModule for WasmtimeRuntime {
@@ -95,6 +147,7 @@ impl WasmModule for WasmtimeRuntime {
 			InternalInstantiationStrategy::Builtin => Strategy::RecreateInstance(InstanceCreator {
 				engine: self.engine.clone(),
 				instance_pre: self.instance_pre.clone(),
+				instance_counter: self.instance_counter.clone(),
 			}),
 		};
 
@@ -277,7 +330,7 @@ fn common_config(semantics: &Semantics) -> std::result::Result<wasmtime::Config,
 			.instance_memories(1)
 			// This determines how many instances of the module can be
 			// instantiated in parallel from the same `Module`.
-			.instance_count(32);
+			.instance_count(MAX_INSTANCE_COUNT);
 
 		config.allocation_strategy(wasmtime::InstanceAllocationStrategy::Pooling(pooling_config));
 	}
@@ -587,7 +640,12 @@ where
 		.instantiate_pre(&module)
 		.map_err(|e| WasmError::Other(format!("cannot preinstantiate module: {:#}", e)))?;
 
-	Ok(WasmtimeRuntime { engine, instance_pre: Arc::new(instance_pre), instantiation_strategy })
+	Ok(WasmtimeRuntime {
+		engine,
+		instance_pre: Arc::new(instance_pre),
+		instantiation_strategy,
+		instance_counter: Default::default(),
+	})
 }
 
 fn prepare_blob_for_compilation(