From e2b21d00c7856961ab3b692b2e2798747af1e819 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de>
Date: Sun, 22 Oct 2023 10:44:34 +0200
Subject: [PATCH] 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 | 1 +
.../client/cli/src/params/runtime_params.rs | 6 +-
substrate/client/executor/wasmtime/Cargo.toml | 1 +
.../executor/wasmtime/src/instance_wrapper.rs | 17 ++++-
.../client/executor/wasmtime/src/runtime.rs | 64 ++++++++++++++++++-
5 files changed, 80 insertions(+), 9 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 1ea3eb38d0c..ae88e4c2c99 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -15172,6 +15172,7 @@ dependencies = [
"libc",
"log",
"parity-scale-codec",
+ "parking_lot 0.12.1",
"paste",
"rustix 0.36.15",
"sc-allocator",
diff --git a/substrate/client/cli/src/params/runtime_params.rs b/substrate/client/cli/src/params/runtime_params.rs
index 79035fc7d4c..07009a96ee6 100644
--- a/substrate/client/cli/src/params/runtime_params.rs
+++ b/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)
}
diff --git a/substrate/client/executor/wasmtime/Cargo.toml b/substrate/client/executor/wasmtime/Cargo.toml
index fee1afc9666..261d52c0ede 100644
--- a/substrate/client/executor/wasmtime/Cargo.toml
+++ b/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!
diff --git a/substrate/client/executor/wasmtime/src/instance_wrapper.rs b/substrate/client/executor/wasmtime/src/instance_wrapper.rs
index acc799061c2..8852532adbc 100644
--- a/substrate/client/executor/wasmtime/src/instance_wrapper.rs
+++ b/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.
diff --git a/substrate/client/executor/wasmtime/src/runtime.rs b/substrate/client/executor/wasmtime/src/runtime.rs
index ae78137959b..ac88663f4e7 100644
--- a/substrate/client/executor/wasmtime/src/runtime.rs
+++ b/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(
--
GitLab