-
Notifications
You must be signed in to change notification settings - Fork 654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perf idea: better runtime support for contracts that use less memory #11033
Comments
What we could do would be to cap the initial memory size to 64MiB, and to use the contract-provided initial memory size if it’s below. The good thing is, so long as the contract is properly compiled, it should not break any contract. The main drawback is implementation complexity (we currently basically do not read the value we replace, meaning we can’t hack around to figure out performance). Also, due to the huge slowdown zeroing out large memories, we would need some way to unmap/remap the memory pages when the memory actually used is too large, instead of zeroing it out, or we’d be vulnerable to huge undercharging attacks. So, to keep in mind if/when we come back to this:
|
Current plan:
|
It looks like all the contracts from our top 10 most gas-using accounts request exactly 17 pages of initial wasm memory. My guess is this is the current minimum implied by rustc, assuming the contracts are all written in rust. So I reran the experiment from #10851, except with a 20-pages hardcoded initial memory at the 3 places the #10851 protocol change patch had a 1 hardcoded. This is still very much a hack, but should be enough to at least get a rough answer of whether there’s actually gains to be hoped for by implementing the full feature. The results are as follow:
So there does seem to be some pretty significant gains to be had here. However, we will need a very good plan to test the changes, because of the results from shard 2 and 3 proving that there are lots of risks if the change is ill-implemented (like the hackish change used to test the potential gains) Current plan:
|
<p>This PR was automatically created by Snyk using the credentials of a real user.</p><br /><h3>Snyk has created this PR to upgrade react-router from 6.20.1 to 6.21.0.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **5 versions** ahead of your current version. - The recommended version was released **21 days ago**, on 2023-12-13. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>react-router</b></summary> <ul> <li> <b>6.21.0</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.21.0">2023-12-13</a></br><p>react-router-native@6.21.0</p> </li> <li> <b>6.21.0-pre.3</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.21.0-pre.3">2023-12-06</a></br><p>react-router-native@6.21.0-pre.3</p> </li> <li> <b>6.21.0-pre.2</b> - 2023-12-05 </li> <li> <b>6.21.0-pre.1</b> - 2023-12-05 </li> <li> <b>6.21.0-pre.0</b> - 2023-12-05 </li> <li> <b>6.20.1</b> - 2023-12-01 </li> </ul> from <a href="https://snyk.io/redirect/github/remix-run/react-router/releases">react-router GitHub release notes</a> </details> </details> <details> <summary><b>Commit messages</b></summary> </br> <details> <summary>Package name: <b>react-router</b></summary> <ul> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/69ba50e06633fd4add234fb47f2d49b0a5ee41f9">69ba50e</a> chore: Update version for release (near#11114)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ffd4373737a5bda3ad0fcebfa4895fbd8038a504">ffd4373</a> Exit prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ea0ffeef8a0c353f8ef402b5df2ac7c60a4d0b49">ea0ffee</a> chore: Update version for release (pre) (near#11095)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/fe3c071037b18f447dabb44fb24e4b1bce2a2a15">fe3c071</a> Slight refactor to partial hydration to leverage state.initialized properly (near#11094)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ed40ee9e61f3c4608bcc2e4cb1275c4b5dbd144e">ed40ee9</a> Update release notes comparison link</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1447eb03a740b765ce79079ebd832afb5d1b02d8">1447eb0</a> Update release notes</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/373b30cdfb4a770e77f6ded25e94ba72fbde4e23">373b30c</a> chore: Update version for release (pre) (near#11091)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f9d7ed62904766013d05a1642232d73e47f3bc27">f9d7ed6</a> Fix server future plumbing</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/56b2944ef18307fa6d5e432b52eead38eeda3402">56b2944</a> chore: Update version for release (pre) (near#11090)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/558d7936cc8fe643374a2c9a9fdcf022e8c4c939">558d793</a> Fix plumbing of future prop</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1c6f3b31fcd49a9065298769d097df420216238e">1c6f3b3</a> fix typos</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ee5fcd54af4784d58191d97a7117fa4d75378946">ee5fcd5</a> Generate release notes</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ddc2b94c26ac4a12b5a8e37dc4179e1b10644e1b">ddc2b94</a> chore: Update version for release (pre) (near#11089)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1752d84eb44f9e1218798a1e431be9b02bbef9ab">1752d84</a> Enter prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/120de8c6c61af165cd09b3b785141336619aadf8">120de8c</a> Merge branch 'main' into release-next</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/149ad65a8dfb90b18835ec784792b6a86e4427a9">149ad65</a> Add future.v7_relativeSplatPath flag (near#11087)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/839df232462e710f5f3bdff1cac2dff9504820be">839df23</a> Bump bundle</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/d04c54100c527dd84c1ca4b7059d38d2a4b4be99">d04c541</a> Fix tests from conflicting PRs</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/0d2a38c7395d2434f9b8cf8100890b8ad6aad0b8">0d2a38c</a> Support partial hydration for Remix clientLoader/clientAction (near#11033)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/cb53f41d4341fcc7ca053d89b5bda03a4fe95106">cb53f41</a> Fix issue when rendering Link/NavLink outside of matched routes (near#11062)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/211c1ff4af0fbf18d79de71a59dd1b6fe410637e">211c1ff</a> Bump bundle</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/e6814d5202beab18a259441144203727a3f13287">e6814d5</a> Properly handle falsy error values in ErrorBoundary's (near#11071)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/dc7833c2beec923a26f2e54898f66fe81e524fa5">dc7833c</a> Catch errors when trying to unwrap responses (near#11061)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/7a5c0a7d91b6f62847418a46ec2d43e0e292a5b2">7a5c0a7</a> Merge branch 'release-next' into dev</li> </ul> <a href="https://snyk.io/redirect/github/remix-run/react-router/compare/8b1ee67ebc00fc3e778d5e36fe9bca140b576b5c...69ba50e06633fd4add234fb47f2d49b0a5ee41f9">Compare</a> </details> </details> <hr/> **Note:** *You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs.* For more information: <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiJhNDI2MjMwYi1iOTMzLTQwZmYtYjU1YS1mM2U2YTM5MzE2ODciLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6ImE0MjYyMzBiLWI5MzMtNDBmZi1iNTVhLWYzZTZhMzkzMTY4NyJ9fQ==" width="0" height="0"/> 🧐 [View latest project report](https://app.snyk.io/org/near-ecosystem/project/e3526cdd-987a-4d84-a55d-30d275fc17bc?utm_source=github&utm_medium=referral&page=upgrade-pr) 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/near-ecosystem/project/e3526cdd-987a-4d84-a55d-30d275fc17bc/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/near-ecosystem/project/e3526cdd-987a-4d84-a55d-30d275fc17bc/settings/integration?pkg=react-router&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) <!--- (snyk:metadata:{"prId":"a426230b-b933-40ff-b55a-f3e6a3931687","prPublicId":"a426230b-b933-40ff-b55a-f3e6a3931687","dependencies":[{"name":"react-router","from":"6.20.1","to":"6.21.0"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/near-ecosystem/project/e3526cdd-987a-4d84-a55d-30d275fc17bc?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"e3526cdd-987a-4d84-a55d-30d275fc17bc","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":5,"publishedDate":"2023-12-13T21:34:17.308Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]}) ---> Co-authored-by: snyk-bot <snyk-bot@snyk.io>
This is basically just moving code around, to be able to reduce visibility into NearVmMemory’s internals and thus derisk modifying them. Part of #11033
TL;DRBenchmark results of the full implementation on our current workloadI finished implementing the full change (minus the "protocol version bump" part of it) and benchmarked what the speedup would be if it were to be fully activated. The results are unfortunately as I initially feared: the time spent in MMAP is actually mostly zeroing out memory, and thus we cannot optimize it out. Actually, this change results in a 20~30% slowdown even in the best-case scenario where we know what memory our contracts request ahead of time. The hope for speedup that came from the hackish experiment results was actually due to it being hackish, and likely not resetting properly the whole memory, or just resulting in contract crashes. Further work that could happen on this topicWe could maybe hope for some improvements if contracts did active work to reduce their initial memory requirements, but they currently all request ~1MiB, due to rustc defaults. We could ask them to compile with Then we could maybe hope for the ~10% performance win that could be seen. Or maybe we would still see the 20-30% performance slowdown that we saw with 17 initial pages. In order to test we’d need some reproducible workload on a contract that we control, and that we could compile with varying numbers of memory pages. In order to properly implement this, we would also need some incentive for contract authors to care about this rustc flag, and thus a new gas cost that’d depend on the size of the initially-requested memory. This would, again, be quite a lot of additional effort. I’ve been suggesting dropping the idea before we even started to initially work on it due to the fact that the linux kernel is often pretty well-implemented, and it has access to more information than we could have as it knows which pages have actually been touched. So I’m once more suggesting that we drop the idea, because mmap should have minimal overhead besides just its memory zeroing, and we cannot just not do the memory zeroing. If we ever have such a reproducible benchmark ready then it might make sense to at least see whether the patch does bring in benefits or not. But until then I don’t think it makes sense to spend more time on this optimization idea. Detailed resultsControl groupThe control group, Patchcommit d91e1d9bd71fc3f544e2d3b3c2dcb0a65542fc24
Author: Léo Gaspard <leo@near.org>
Date: Wed Apr 24 10:34:23 2024 +0000
wip
diff --git a/Cargo.lock b/Cargo.lock
index 9b7f3bf9d..674bcf206 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4929,11 +4929,13 @@ dependencies = [
"borsh 1.0.0",
"bytesize",
"cov-mark",
+ "crossbeam",
"ed25519-dalek",
"enum-map",
"expect-test",
"finite-wasm",
"hex",
+ "lazy_static",
"lru 0.12.3",
"memoffset 0.8.0",
"near-crypto",
diff --git a/core/parameters/src/parameter_table.rs b/core/parameters/src/parameter_table.rs
index b4d68efb2..0b9a8fb5c 100644
--- a/core/parameters/src/parameter_table.rs
+++ b/core/parameters/src/parameter_table.rs
@@ -326,6 +326,7 @@ impl TryFrom<&ParameterTable> for RuntimeConfig {
function_call_weight: params.get(Parameter::FunctionCallWeight)?,
eth_implicit_accounts: params.get(Parameter::EthImplicitAccounts)?,
yield_resume_host_functions: params.get(Parameter::YieldResume)?,
+ lower_initial_contract_memory: true, // TODO: protocol change
},
account_creation_config: AccountCreationConfig {
min_allowed_top_level_account_length: params
diff --git a/core/parameters/src/view.rs b/core/parameters/src/view.rs
index 39487c249..06a6d656c 100644
--- a/core/parameters/src/view.rs
+++ b/core/parameters/src/view.rs
@@ -227,6 +227,8 @@ pub struct VMConfigView {
pub eth_implicit_accounts: bool,
/// See [`VMConfig::yield_resume_host_functions`].
pub yield_resume_host_functions: bool,
+ /// See [`VMConfig::lower_initial_contract_memory`].
+ pub lower_initial_contract_memory: bool,
/// Describes limits for VM and Runtime.
///
@@ -253,6 +255,7 @@ impl From<crate::vm::Config> for VMConfigView {
vm_kind: config.vm_kind,
eth_implicit_accounts: config.eth_implicit_accounts,
yield_resume_host_functions: config.yield_resume_host_functions,
+ lower_initial_contract_memory: config.lower_initial_contract_memory,
}
}
}
@@ -275,6 +278,7 @@ impl From<VMConfigView> for crate::vm::Config {
vm_kind: view.vm_kind,
eth_implicit_accounts: view.eth_implicit_accounts,
yield_resume_host_functions: view.yield_resume_host_functions,
+ lower_initial_contract_memory: view.lower_initial_contract_memory,
}
}
}
diff --git a/core/parameters/src/vm.rs b/core/parameters/src/vm.rs
index f2f7a46af..7c721e60f 100644
--- a/core/parameters/src/vm.rs
+++ b/core/parameters/src/vm.rs
@@ -192,6 +192,9 @@ pub struct Config {
/// Enable the `promise_yield_create` and `promise_yield_resume` host functions.
pub yield_resume_host_functions: bool,
+ /// Enable the `LowerInitialContractMemory` protocol feature.
+ pub lower_initial_contract_memory: bool,
+
/// Describes limits for VM and Runtime.
pub limit_config: LimitConfig,
}
@@ -224,6 +227,7 @@ impl Config {
self.ed25519_verify = true;
self.math_extension = true;
self.implicit_account_creation = true;
+ self.lower_initial_contract_memory = true;
}
}
diff --git a/runtime/near-vm-runner/Cargo.toml b/runtime/near-vm-runner/Cargo.toml
index 042bd0bab..2e5910369 100644
--- a/runtime/near-vm-runner/Cargo.toml
+++ b/runtime/near-vm-runner/Cargo.toml
@@ -17,9 +17,11 @@ anyhow = { workspace = true, optional = true }
base64.workspace = true
bn.workspace = true
borsh.workspace = true
+crossbeam.workspace = true
ed25519-dalek.workspace = true
enum-map.workspace = true
finite-wasm = { workspace = true, features = ["instrument"], optional = true }
+lazy_static.workspace = true
lru = "0.12.3"
memoffset.workspace = true
num-rational.workspace = true
diff --git a/runtime/near-vm-runner/src/cache.rs b/runtime/near-vm-runner/src/cache.rs
index 11b238a66..ae4c19c25 100644
--- a/runtime/near-vm-runner/src/cache.rs
+++ b/runtime/near-vm-runner/src/cache.rs
@@ -80,6 +80,7 @@ impl CompiledContract {
#[derive(Debug, Clone, PartialEq, BorshDeserialize, BorshSerialize)]
pub struct CompiledContractInfo {
pub wasm_bytes: u64,
+ pub initial_memory_pages: u32,
pub compiled: CompiledContract,
}
@@ -314,6 +315,7 @@ impl ContractRuntimeCache for FilesystemContractRuntimeCache {
}
}
temp_file.write_all(&value.wasm_bytes.to_le_bytes())?;
+ temp_file.write_all(&value.initial_memory_pages.to_le_bytes())?;
let temp_filename = temp_file.into_temp_path();
// This is atomic, so there wouldn't be instances where getters see an intermediate state.
rustix::fs::renameat(&self.state.dir, &*temp_filename, &self.state.dir, final_filename)?;
@@ -351,15 +353,17 @@ impl ContractRuntimeCache for FilesystemContractRuntimeCache {
// The file turns out to be empty/truncated? Treat as if there's no cached file.
return Ok(None);
}
- let wasm_bytes = u64::from_le_bytes(buffer[buffer.len() - 8..].try_into().unwrap());
- let tag = buffer[buffer.len() - 9];
- buffer.truncate(buffer.len() - 9);
+ let initial_memory_pages = u32::from_le_bytes(buffer[buffer.len() - 4..].try_into().unwrap());
+ let wasm_bytes = u64::from_le_bytes(buffer[buffer.len() - 12..buffer.len() - 4].try_into().unwrap());
+ let tag = buffer[buffer.len() - 13];
+ buffer.truncate(buffer.len() - 13);
Ok(match tag {
CODE_TAG => {
- Some(CompiledContractInfo { wasm_bytes, compiled: CompiledContract::Code(buffer) })
+ Some(CompiledContractInfo { wasm_bytes, initial_memory_pages, compiled: CompiledContract::Code(buffer) })
}
ERROR_TAG => Some(CompiledContractInfo {
wasm_bytes,
+ initial_memory_pages,
compiled: CompiledContract::CompileModuleError(borsh::from_slice(&buffer)?),
}),
// File is malformed? For this code, since we're talking about a cache lets just treat
diff --git a/runtime/near-vm-runner/src/near_vm_runner/memory.rs b/runtime/near-vm-runner/src/near_vm_runner/memory.rs
index 7d8b7f570..37432de63 100644
--- a/runtime/near-vm-runner/src/near_vm_runner/memory.rs
+++ b/runtime/near-vm-runner/src/near_vm_runner/memory.rs
@@ -26,7 +26,6 @@ impl NearVmMemory {
)?)))
}
- #[cfg(unused)] // TODO: this will be used once we reuse the memories
pub fn into_preallocated(self) -> Result<PreallocatedMemory, String> {
Ok(PreallocatedMemory(
Arc::into_inner(self.0)
diff --git a/runtime/near-vm-runner/src/near_vm_runner/mod.rs b/runtime/near-vm-runner/src/near_vm_runner/mod.rs
index f2c7b48b5..331d2e407 100644
--- a/runtime/near-vm-runner/src/near_vm_runner/mod.rs
+++ b/runtime/near-vm-runner/src/near_vm_runner/mod.rs
@@ -38,7 +38,7 @@ enum NearVmCompiler {
// major version << 6
// minor version
const VM_CONFIG: NearVmConfig = NearVmConfig {
- seed: (2 << 29) | (2 << 6) | 2,
+ seed: (2 << 29) | (2 << 6) | 15,
engine: NearVmEngine::Universal,
compiler: NearVmCompiler::Singlepass,
};
diff --git a/runtime/near-vm-runner/src/near_vm_runner/runner.rs b/runtime/near-vm-runner/src/near_vm_runner/runner.rs
index eb0cdf12d..2e6f89e35 100644
--- a/runtime/near-vm-runner/src/near_vm_runner/runner.rs
+++ b/runtime/near-vm-runner/src/near_vm_runner/runner.rs
@@ -8,6 +8,7 @@ use crate::logic::errors::{
use crate::logic::gas_counter::FastGasCounter;
use crate::logic::types::PromiseResult;
use crate::logic::{Config, External, VMContext, VMLogic, VMOutcome};
+use crate::near_vm_runner::memory::PreallocatedMemory;
use crate::near_vm_runner::{NearVmCompiler, NearVmEngine};
use crate::runner::VMResult;
use crate::{
@@ -160,7 +161,7 @@ impl NearVM {
pub(crate) fn compile_uncached(
&self,
code: &ContractCode,
- ) -> Result<UniversalExecutable, CompilationError> {
+ ) -> Result<(u32, UniversalExecutable), CompilationError> {
let _span = tracing::debug_span!(target: "vm", "NearVM::compile_uncached").entered();
let prepared_code = prepare::prepare_contract(code.code(), &self.config, VMKind::NearVm)
.map_err(CompilationError::PrepareError)?;
@@ -169,6 +170,7 @@ impl NearVM {
matches!(self.engine.validate(&prepared_code), Ok(_)),
"near_vm failed to validate the prepared code"
);
+ let initial_memory_pages = self.initial_memory_pages_for(&prepared_code)?;
let executable = self
.engine
.compile_universal(&prepared_code, &self)
@@ -176,19 +178,20 @@ impl NearVM {
tracing::error!(?err, "near_vm failed to compile the prepared code (this is defense-in-depth, the error was recovered from but should be reported to pagoda)");
CompilationError::WasmerCompileError { msg: err.to_string() }
})?;
- Ok(executable)
+ Ok((initial_memory_pages, executable))
}
fn compile_and_cache(
&self,
code: &ContractCode,
cache: &dyn ContractRuntimeCache,
- ) -> Result<Result<UniversalExecutable, CompilationError>, CacheError> {
+ ) -> Result<Result<(u32, UniversalExecutable), CompilationError>, CacheError> {
let executable_or_error = self.compile_uncached(code);
let key = get_contract_cache_key(*code.hash(), &self.config);
let record = CompiledContractInfo {
wasm_bytes: code.code().len() as u64,
- compiled: match &executable_or_error {
+ initial_memory_pages: executable_or_error.as_ref().map(|r| r.0).unwrap_or(self.config.limit_config.initial_memory_pages),
+ compiled: match executable_or_error.as_ref().map(|r| &r.1) {
Ok(executable) => {
let code = executable
.serialize()
@@ -220,10 +223,10 @@ impl NearVM {
method_name: &str,
closure: impl FnOnce(VMMemory, VMLogic<'_>, &VMArtifact) -> Result<VMOutcome, VMRunnerError>,
) -> VMResult<VMOutcome> {
- // (wasm code size, compilation result)
- type MemoryCacheType = (u64, Result<VMArtifact, CompilationError>);
+ // (wasm code size, initial memory pages, compilation result)
+ type MemoryCacheType = (u64, u32, Result<VMArtifact, CompilationError>);
let to_any = |v: MemoryCacheType| -> Box<dyn std::any::Any + Send> { Box::new(v) };
- let (wasm_bytes, artifact_result) = cache.memory_cache().try_lookup(
+ let (wasm_bytes, initial_memory_pages, artifact_result) = cache.memory_cache().try_lookup(
code_hash,
|| match code {
None => {
@@ -243,9 +246,9 @@ impl NearVM {
};
match &code.compiled {
- CompiledContract::CompileModuleError(err) => {
- Ok::<_, VMRunnerError>(to_any((code.wasm_bytes, Err(err.clone()))))
- }
+ CompiledContract::CompileModuleError(err) => Ok::<_, VMRunnerError>(
+ to_any((code.wasm_bytes, code.initial_memory_pages, Err(err.clone()))),
+ ),
CompiledContract::Code(serialized_module) => {
let _span =
tracing::debug_span!(target: "vm", "NearVM::load_from_fs_cache")
@@ -269,7 +272,11 @@ impl NearVM {
.load_universal_executable_ref(&executable)
.map(Arc::new)
.map_err(|err| VMRunnerError::LoadingError(err.to_string()))?;
- Ok(to_any((code.wasm_bytes, Ok(artifact))))
+ Ok(to_any((
+ code.wasm_bytes,
+ code.initial_memory_pages,
+ Ok(artifact),
+ )))
}
}
}
@@ -277,34 +284,39 @@ impl NearVM {
Some(code) => {
let _span =
tracing::debug_span!(target: "vm", "NearVM::build_from_source").entered();
- Ok(to_any((
- code.code().len() as u64,
+ let (initial_memory_pages, compiled) =
match self.compile_and_cache(code, cache)? {
- Ok(executable) => Ok(self
- .engine
- .load_universal_executable(&executable)
- .map(Arc::new)
- .map_err(|err| VMRunnerError::LoadingError(err.to_string()))?),
- Err(err) => Err(err),
- },
- )))
+ Ok((initial_memory_pages, executable)) => (
+ initial_memory_pages,
+ Ok(self
+ .engine
+ .load_universal_executable(&executable)
+ .map(Arc::new)
+ .map_err(|err| VMRunnerError::LoadingError(err.to_string()))?),
+ ),
+ Err(err) => (0, Err(err)),
+ };
+ Ok(to_any((code.code().len() as u64, initial_memory_pages, compiled)))
}
},
move |value| {
let _span =
tracing::debug_span!(target: "vm", "NearVM::load_from_mem_cache").entered();
- let &(wasm_bytes, ref downcast) = value
+ let &(wasm_bytes, initial_memory_pages, ref downcast) = value
.downcast_ref::<MemoryCacheType>()
.expect("downcast should always succeed");
- (wasm_bytes, downcast.clone())
+ (wasm_bytes, initial_memory_pages, downcast.clone())
},
)?;
+ lazy_static::lazy_static! {
+ static ref MEMORIES: crossbeam::queue::ArrayQueue<PreallocatedMemory> = crossbeam::queue::ArrayQueue::new(16);
+ }
let mut memory = NearVmMemory::new(
- self.config.limit_config.initial_memory_pages,
+ initial_memory_pages,
self.config.limit_config.max_memory_pages,
- None, // TODO: this should actually reuse the memories
+ MEMORIES.pop(),
)
.expect("Cannot create memory for a contract call");
// FIXME: this mostly duplicates the `run_module` method.
@@ -323,12 +335,38 @@ impl NearVM {
if let Err(e) = result {
return Ok(VMOutcome::abort(logic, e));
}
- closure(vmmemory, logic, &artifact)
+ let res = closure(vmmemory, logic, &artifact);
+ if let Ok(mmap) = memory.into_preallocated() {
+ tracing::info!("Reusing a memory");
+ let _ = MEMORIES.push(mmap);
+ } else {
+ tracing::error!("Not reusing a memory")
+ }
+ res
}
Err(e) => Ok(VMOutcome::abort(logic, FunctionCallError::CompilationError(e))),
}
}
+ fn initial_memory_pages_for(&self, code: &[u8]) -> Result<u32, CompilationError> {
+ let parser = wasmparser::Parser::new(0);
+ for payload in parser.parse_all(code) {
+ if let Ok(wasmparser::Payload::ImportSection(reader)) = payload {
+ for mem in reader {
+ if let Ok(mem) = mem {
+ if let wasmparser::ImportSectionEntryType::Memory(
+ wasmparser::MemoryType::M32 { limits, .. },
+ ) = mem.ty
+ {
+ return Ok(limits.initial);
+ }
+ }
+ }
+ }
+ }
+ panic!("Tried running a contract that was not prepared with a memory import");
+ }
+
fn run_method(
&self,
artifact: &VMArtifact,
diff --git a/runtime/near-vm-runner/src/prepare/prepare_v2.rs b/runtime/near-vm-runner/src/prepare/prepare_v2.rs
index 894dea6fd..ada52e31b 100644
--- a/runtime/near-vm-runner/src/prepare/prepare_v2.rs
+++ b/runtime/near-vm-runner/src/prepare/prepare_v2.rs
@@ -240,8 +240,37 @@ impl<'a> PrepareContext<'a> {
}
fn memory_import(&self) -> wasm_encoder::EntityType {
+ // First, figure out the requested initial memory
+ // This parsing should be fast enough, as it can skip over whole sections
+ // And considering the memory section is after the import section, we will not
+ // have parsed it yet in the "regular" parse when calling this function.
+ let mut requested_initial_memory = None;
+ let parser = wp::Parser::new(0);
+ 'outer: for payload in parser.parse_all(self.code) {
+ if let Ok(wp::Payload::MemorySection(reader)) = payload {
+ for mem in reader {
+ if let Ok(mem) = mem {
+ if requested_initial_memory.is_some() {
+ requested_initial_memory = None;
+ break 'outer;
+ }
+ requested_initial_memory = Some(mem.initial);
+ }
+ }
+ }
+ }
+
+ // Then, generate a memory import, that has at most the limit-configured initial memory,
+ // but tries to get that number down by using the contract-provided data.
+ let max_initial_memory = requested_initial_memory.unwrap_or(u64::MAX);
+ let config_initial_memory = u64::from(self.config.limit_config.initial_memory_pages);
+ let initial_memory = if self.config.lower_initial_contract_memory {
+ std::cmp::min(max_initial_memory, config_initial_memory)
+ } else {
+ config_initial_memory
+ };
wasm_encoder::EntityType::Memory(wasm_encoder::MemoryType {
- minimum: u64::from(self.config.limit_config.initial_memory_pages),
+ minimum: initial_memory,
maximum: Some(u64::from(self.config.limit_config.max_memory_pages)),
memory64: false,
shared: false,
diff --git a/runtime/near-vm-runner/src/wasmer2_runner.rs b/runtime/near-vm-runner/src/wasmer2_runner.rs
index d79e6b1cd..c7f952f56 100644
--- a/runtime/near-vm-runner/src/wasmer2_runner.rs
+++ b/runtime/near-vm-runner/src/wasmer2_runner.rs
@@ -300,6 +300,7 @@ impl Wasmer2VM {
if let Some(cache) = cache {
let record = CompiledContractInfo {
wasm_bytes: code.code().len() as u64,
+ initial_memory_pages: self.config.limit_config.initial_memory_pages,
compiled: match &executable_or_error {
Ok(executable) => {
let code = executable
diff --git a/runtime/near-vm-runner/src/wasmer_runner.rs b/runtime/near-vm-runner/src/wasmer_runner.rs
index 59bf3bf81..f2ea9f06d 100644
--- a/runtime/near-vm-runner/src/wasmer_runner.rs
+++ b/runtime/near-vm-runner/src/wasmer_runner.rs
@@ -268,6 +268,7 @@ impl Wasmer0VM {
if let Some(cache) = cache {
let record = CompiledContractInfo {
wasm_bytes: code.code().len() as u64,
+ initial_memory_pages: self.config.limit_config.initial_memory_pages,
compiled: match &module_or_error {
Ok(module) => {
let code = module
diff --git a/runtime/near-vm/vm/src/memory/linear_memory.rs b/runtime/near-vm/vm/src/memory/linear_memory.rs
index 6e786e00c..43f84b8bc 100644
--- a/runtime/near-vm/vm/src/memory/linear_memory.rs
+++ b/runtime/near-vm/vm/src/memory/linear_memory.rs
@@ -141,7 +141,7 @@ impl LinearMemory {
let mapped_pages = memory.minimum;
let mapped_bytes = mapped_pages.bytes();
- let alloc = if let Some(alloc) = from_mmap {
+ let alloc = if let Some(mut alloc) = from_mmap {
// For now we always request the same size, because our prepare step hardcodes a maximum size
// of 64 MiB. This could change in the future, at which point this assert will start triggering
// and we’ll need to think of a better way to handle things.
@@ -150,6 +150,7 @@ impl LinearMemory {
request_bytes,
"Multiple data memory mmap's had different maximal lengths"
);
+ alloc.make_accessible(mapped_bytes.0).map_err(MemoryError::Region)?;
alloc
} else {
Mmap::accessible_reserved(mapped_bytes.0, request_bytes).map_err(MemoryError::Region)?
diff --git a/runtime/near-vm/vm/src/mmap.rs b/runtime/near-vm/vm/src/mmap.rs
index 9df6e1e65..b5985454b 100644
--- a/runtime/near-vm/vm/src/mmap.rs
+++ b/runtime/near-vm/vm/src/mmap.rs
@@ -227,6 +227,9 @@ impl Mmap {
pub fn reset(&mut self) -> Result<(), String> {
unsafe {
if self.accessible_len > 0 {
+ if self.accessible_len > 18 * 64 * 1024 {
+ return Err(String::from("too big memories are not worth resetting"));
+ }
self.as_mut_ptr().write_bytes(0, self.accessible_len);
region::protect(self.as_ptr(), self.accessible_len, region::Protection::NONE)
.map_err(|e| e.to_string())?; TestingRunning the patched neard manually confirmed that the new tracing logs do confirm that memories are properly reused most of the time. In the benchmark results, 17% of the time is also spent in the Running with the Considering this testing, the benchmark results are representative of exactly the performance changes provoked by not relying on the linux kernel’s zeroing out of memory in mmap, and instead doing it ourselves. Benchmark resultsShard 0Shard 0 failed to reproduce blocks, thus preventing a proper benchmark comparison. This is likely due to the missing protocol version bump, and does indicate that finishing the change properly would likely result in real-world contract breakage somewhere on the shard. Shard 1On shard 1, performance is basically the same with and without the patch:
Shard 2Shard 2 sees a 20% slowdown with the patch:
Shard 3Shard 3 sees the same 20-25% slowdown with the patch:
Shard 4Shard 4 was like shard 0, with no results due to outcome changes that break the benchmarking setup. Shard 5Shard 5 even saw the patch-using binary being 35% slower than the baseline:
|
Closing as per the above comment. |
Our runtime today starts contracts with a memory of 64 MiB regardless of what the contract declares. The contract can then bring that number up to 128 MiB. We suspect that many contracts are actually using much less memory than this 64MiB. If someone were to write a contract that uses as little memory as possible (by specifying a small initial
memory
and usingmemory.grow
as appropriate), then maybe we can optimise our runtime to execute such contracts more efficiently.See also #10851 for a preliminary investigation on the matter.
The text was updated successfully, but these errors were encountered: