Skip to content
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

Tracking/limiting memory allocator #1192

Merged
merged 62 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
47ecf58
Import changes from archived repo
s0me0ne-unkn0wn Aug 27, 2023
58330de
Fix dependencies
s0me0ne-unkn0wn Aug 30, 2023
aac1da6
Merge branch 'master' into s0me0ne/tracking-allocator
pepoviola Aug 30, 2023
99d4f5e
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 1, 2023
18ef342
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 2, 2023
facd26a
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 12, 2023
3d2146a
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 14, 2023
6ee5ff3
Format manifests
s0me0ne-unkn0wn Sep 14, 2023
f85c753
Make peak allocation value observable
s0me0ne-unkn0wn Sep 15, 2023
c9c0bf8
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 15, 2023
edfa2e1
Enforce memory limits
s0me0ne-unkn0wn Sep 16, 2023
ac5dab4
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 16, 2023
994273a
Fix the node allocator declaration
s0me0ne-unkn0wn Sep 16, 2023
72ded39
`cargo fmt`
s0me0ne-unkn0wn Sep 16, 2023
dfa0daf
Implement abort handler
s0me0ne-unkn0wn Sep 21, 2023
e740b5c
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 21, 2023
48cde3e
Address some discussions
s0me0ne-unkn0wn Sep 21, 2023
acf149e
Remove feature gate
s0me0ne-unkn0wn Sep 21, 2023
6cfeceb
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 21, 2023
c7fa464
Update crate description
s0me0ne-unkn0wn Sep 22, 2023
edeb901
Implement failure callback
s0me0ne-unkn0wn Sep 22, 2023
251ac90
Merge remote-tracking branch 'origin/s0me0ne/tracking-allocator' into…
s0me0ne-unkn0wn Sep 22, 2023
4195806
Address discussions
s0me0ne-unkn0wn Sep 22, 2023
09e5c07
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 22, 2023
02ad536
Fix tests
s0me0ne-unkn0wn Sep 22, 2023
e1a2b63
Fix typo
s0me0ne-unkn0wn Sep 25, 2023
1918d2a
Address discussions
s0me0ne-unkn0wn Sep 26, 2023
b06cb6d
Merge remote-tracking branch 'origin/s0me0ne/tracking-allocator' into…
s0me0ne-unkn0wn Sep 26, 2023
39cf3b7
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 26, 2023
007054a
Address discussions
s0me0ne-unkn0wn Sep 26, 2023
c81e8aa
Try to fix test pipeline
s0me0ne-unkn0wn Sep 26, 2023
5920260
Fix PVF preparation benchmark
s0me0ne-unkn0wn Sep 26, 2023
a37c688
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 26, 2023
96d71c9
Minor fixes
s0me0ne-unkn0wn Sep 27, 2023
30e5236
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 27, 2023
73799fb
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 28, 2023
93b9d61
Eliminate redundant branch
s0me0ne-unkn0wn Sep 28, 2023
8485741
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 28, 2023
7a3875a
Revert "Eliminate redundant branch"
s0me0ne-unkn0wn Sep 28, 2023
83e6b99
Remove redundant todo comment
s0me0ne-unkn0wn Sep 28, 2023
6c6afc8
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Sep 29, 2023
9de0856
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Oct 4, 2023
d6470c5
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Oct 5, 2023
d2d8a60
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Oct 11, 2023
d74aa20
Remove stale file
s0me0ne-unkn0wn Oct 11, 2023
fb40a0d
Merge branch 's0me0ne/tracking-allocator' of github.com:paritytech/po…
s0me0ne-unkn0wn Oct 20, 2023
9c1d920
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Oct 20, 2023
c6be92b
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Oct 23, 2023
8c58a68
Abstract `Spinlock`
s0me0ne-unkn0wn Oct 23, 2023
52578ee
Merge branch 'master' into s0me0ne/tracking-allocator
s0me0ne-unkn0wn Oct 23, 2023
778513c
Update comment
s0me0ne-unkn0wn Oct 29, 2023
9c3ea9b
Add safety comment
s0me0ne-unkn0wn Oct 29, 2023
54686a1
Fix comment formatting
s0me0ne-unkn0wn Oct 29, 2023
07eb6a9
Fix typo
s0me0ne-unkn0wn Oct 29, 2023
e3ca3b6
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Oct 29, 2023
8be6359
Remove unneeded trait bound
s0me0ne-unkn0wn Oct 29, 2023
8b0a722
Remove stale file
s0me0ne-unkn0wn Oct 29, 2023
331ed01
Rename tracking fn
s0me0ne-unkn0wn Nov 1, 2023
7d69a9b
Merge remote-tracking branch 'origin/master' into s0me0ne/tracking-al…
s0me0ne-unkn0wn Nov 1, 2023
10e7443
Fix clippy and tests
s0me0ne-unkn0wn Nov 1, 2023
c3b66f2
Merge branch 'master' into s0me0ne/tracking-allocator
s0me0ne-unkn0wn Nov 1, 2023
e94ddf9
Merge branch 'master' into s0me0ne/tracking-allocator
s0me0ne-unkn0wn Nov 3, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 26 additions & 1 deletion polkadot/node/core/pvf/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,40 +23,56 @@ use std::fmt;
pub type PrepareResult = Result<PrepareStats, PrepareError>;

/// An error that occurred during the prepare part of the PVF pipeline.
// Codec indexes are intended to stabilize pre-encoded payloads (see `OOM_PAYLOAD` below)
#[derive(Debug, Clone, Encode, Decode)]
pub enum PrepareError {
/// During the prevalidation stage of preparation an issue was found with the PVF.
#[codec(index = 0)]
s0me0ne-unkn0wn marked this conversation as resolved.
Show resolved Hide resolved
Prevalidation(String),
/// Compilation failed for the given PVF.
#[codec(index = 1)]
Preparation(String),
/// Instantiation of the WASM module instance failed.
#[codec(index = 2)]
RuntimeConstruction(String),
/// An unexpected panic has occurred in the preparation worker.
#[codec(index = 3)]
Panic(String),
/// Failed to prepare the PVF due to the time limit.
#[codec(index = 4)]
TimedOut,
/// An IO error occurred. This state is reported by either the validation host or by the
/// worker.
#[codec(index = 5)]
IoErr(String),
/// The temporary file for the artifact could not be created at the given cache path. This
/// state is reported by the validation host (not by the worker).
#[codec(index = 6)]
CreateTmpFileErr(String),
/// The response from the worker is received, but the file cannot be renamed (moved) to the
/// final destination location. This state is reported by the validation host (not by the
/// worker).
#[codec(index = 7)]
RenameTmpFileErr {
err: String,
// Unfortunately `PathBuf` doesn't implement `Encode`/`Decode`, so we do a fallible
// conversion to `Option<String>`.
src: Option<String>,
dest: Option<String>,
},
/// Memory limit reached
#[codec(index = 8)]
OutOfMemory,
/// The response from the worker is received, but the worker cache could not be cleared. The
/// worker has to be killed to avoid jobs having access to data from other jobs. This state is
/// reported by the validation host (not by the worker).
#[codec(index = 9)]
ClearWorkerDir(String),
}

/// Pre-encoded length-prefixed `PrepareResult::Err(PrepareError::OutOfMemory)`
pub const OOM_PAYLOAD: &[u8] = b"\x02\x00\x00\x00\x00\x00\x00\x00\x01\x08";

impl PrepareError {
/// Returns whether this is a deterministic error, i.e. one that should trigger reliably. Those
/// errors depend on the PVF itself and the sc-executor/wasmtime logic.
Expand All @@ -67,7 +83,7 @@ impl PrepareError {
pub fn is_deterministic(&self) -> bool {
use PrepareError::*;
match self {
Prevalidation(_) | Preparation(_) | Panic(_) => true,
Prevalidation(_) | Preparation(_) | Panic(_) | OutOfMemory => true,
TimedOut |
IoErr(_) |
CreateTmpFileErr(_) |
Expand All @@ -92,6 +108,7 @@ impl fmt::Display for PrepareError {
CreateTmpFileErr(err) => write!(f, "prepare: error creating tmp file: {}", err),
RenameTmpFileErr { err, src, dest } =>
write!(f, "prepare: error renaming tmp file ({:?} -> {:?}): {}", src, dest, err),
OutOfMemory => write!(f, "prepare: out of memory"),
ClearWorkerDir(err) => write!(f, "prepare: error clearing worker cache: {}", err),
}
}
Expand Down Expand Up @@ -147,3 +164,11 @@ impl fmt::Display for InternalValidationError {
}
}
}

#[test]
fn pre_encoded_payloads() {
let oom_enc = PrepareResult::Err(PrepareError::OutOfMemory).encode();
let mut oom_payload = oom_enc.len().to_le_bytes().to_vec();
oom_payload.extend(oom_enc);
assert_eq!(oom_payload, OOM_PAYLOAD);
}
27 changes: 24 additions & 3 deletions polkadot/node/core/pvf/common/src/executor_intf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,36 @@ pub fn params_to_wasmtime_semantics(par: &ExecutorParams) -> Result<Semantics, S
ExecutorParam::StackLogicalMax(slm) => stack_limit.logical_max = *slm,
ExecutorParam::StackNativeMax(snm) => stack_limit.native_stack_max = *snm,
ExecutorParam::WasmExtBulkMemory => sem.wasm_bulk_memory = true,
// TODO: Not implemented yet; <https://github.com/paritytech/polkadot/issues/6472>.
ExecutorParam::PrecheckingMaxMemory(_) => (),
ExecutorParam::PvfPrepTimeout(_, _) | ExecutorParam::PvfExecTimeout(_, _) => (), /* Not used here */
ExecutorParam::PrecheckingMaxMemory(_) |
ExecutorParam::PvfPrepTimeout(_, _) |
ExecutorParam::PvfExecTimeout(_, _) => (), /* Not used here */
}
}
sem.deterministic_stack_limit = Some(stack_limit);
Ok(sem)
}

/// Runs the prevalidation on the given code. Returns a [`RuntimeBlob`] if it succeeds.
pub fn prevalidate(code: &[u8]) -> Result<RuntimeBlob, sc_executor_common::error::WasmError> {
let blob = RuntimeBlob::new(code)?;
// It's assumed this function will take care of any prevalidation logic
// that needs to be done.
//
// Do nothing for now.
Ok(blob)
}

/// Runs preparation on the given runtime blob. If successful, it returns a serialized compiled
/// artifact which can then be used to pass into `Executor::execute` after writing it to the disk.
pub fn prepare(
blob: RuntimeBlob,
executor_params: &ExecutorParams,
) -> Result<Vec<u8>, sc_executor_common::error::WasmError> {
let semantics = params_to_wasmtime_semantics(executor_params)
.map_err(|e| sc_executor_common::error::WasmError::Other(e))?;
sc_executor_wasmtime::prepare_runtime_artifact(blob, &semantics)
}

/// Available host functions. We leave out:
///
/// 1. storage related stuff (PVF doesn't have a notion of a persistent storage/trie)
Expand Down
4 changes: 3 additions & 1 deletion polkadot/node/core/pvf/common/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ pub struct PrepareStats {
/// supported by the OS, `ru_maxrss`.
#[derive(Clone, Debug, Default, Encode, Decode)]
pub struct MemoryStats {
/// Memory stats from `tikv_jemalloc_ctl`.
/// Memory stats from `tikv_jemalloc_ctl`, polling-based and not very precise.
#[cfg(any(target_os = "linux", feature = "jemalloc-allocator"))]
pub memory_tracker_stats: Option<MemoryAllocationStats>,
/// `ru_maxrss` from `getrusage`. `None` if an error occurred.
#[cfg(target_os = "linux")]
pub max_rss: Option<i64>,
/// Peak allocation in bytes measured by tracking allocator
pub peak_tracked_alloc: u64,
}

/// Statistics of collected memory metrics.
Expand Down
13 changes: 13 additions & 0 deletions polkadot/node/core/pvf/prepare-worker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ cfg-if = "1.0"
gum = { package = "tracing-gum", path = "../../../gum" }
libc = "0.2.139"
rayon = "1.5.1"
tracking-allocator = { path = "../../../tracking-allocator" }
tikv-jemalloc-ctl = { version = "0.5.0", optional = true }
tikv-jemallocator = { version = "0.5.0", optional = true }

parity-scale-codec = { version = "3.6.1", default-features = false, features = ["derive"] }

Expand All @@ -22,11 +24,22 @@ sc-executor-common = { path = "../../../../../substrate/client/executor/common"
sc-executor-wasmtime = { path = "../../../../../substrate/client/executor/wasmtime" }

[target.'cfg(target_os = "linux")'.dependencies]
tikv-jemallocator = "0.5.0"
tikv-jemalloc-ctl = "0.5.0"

[features]
builder = []
jemalloc-allocator = [
"dep:tikv-jemalloc-ctl",
"dep:tikv-jemallocator",
"polkadot-node-core-pvf-common/jemalloc-allocator",
]

[dev-dependencies]
criterion = { version = "0.4.0", default-features = false, features = ["cargo_bench_support"] }
rococo-runtime = { path = "../../../../runtime/rococo" }
sp-maybe-compressed-blob = { path = "../../../../../substrate/primitives/maybe-compressed-blob" }

[[bench]]
name = "prepare_rococo_runtime"
harness = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot 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.

// Polkadot 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 Polkadot. If not, see <http://www.gnu.org/licenses/>.

use criterion::{criterion_group, criterion_main, Criterion, SamplingMode};
use polkadot_node_core_pvf_common::{
executor_intf::{prepare, prevalidate},
prepare::PrepareJobKind,
pvf::PvfPrepData,
};
use polkadot_primitives::ExecutorParams;
use std::time::Duration;

fn do_prepare_runtime(pvf: PvfPrepData) {
let blob = match prevalidate(&pvf.code()) {
Err(err) => panic!("{:?}", err),
Ok(b) => b,
};

match prepare(blob, &pvf.executor_params()) {
Ok(_) => (),
Err(err) => panic!("{:?}", err),
}
}

fn prepare_rococo_runtime(c: &mut Criterion) {
let blob = rococo_runtime::WASM_BINARY.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Use rococo_parachain_runtime instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's not a parachain runtime, it's Rococo runtime itself 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, we're preparing something that doesn't export validate_block and thus can't be executed. It probably doesn't matter for the purpose of the benchmark, but could potentially be a point of confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. But for preparation benchmarking, it's better to use code similar to a real-life parachain code by its volume, and there's nothing suitable at hand. All the test parachains are too simple. So, as we're not going to execute it anyway, I decided to use Rococo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, makes sense. On the other hand, the bench could break if we ever switch to your custom compiler. FWIU, it will either only support compiling PVFs, or at least be optimized specifically for them. Something to keep in mind at least.

let pvf = match sp_maybe_compressed_blob::decompress(&blob, 64 * 1024 * 1024) {
Ok(code) => PvfPrepData::from_code(
code.into_owned(),
ExecutorParams::default(),
Duration::from_secs(360),
PrepareJobKind::Compilation,
),
Err(e) => {
panic!("Cannot decompress blob: {:?}", e);
},
};

let mut group = c.benchmark_group("rococo");
group.sampling_mode(SamplingMode::Flat);
group.sample_size(20);
group.measurement_time(Duration::from_secs(240));
group.bench_function("prepare Rococo runtime", |b| {
// `PvfPrepData` is designed to be cheap to clone, so cloning shouldn't affect the
// benchmark accuracy
b.iter(|| do_prepare_runtime(pvf.clone()))
});
group.finish();
}

criterion_group!(preparation, prepare_rococo_runtime);
criterion_main!(preparation);
42 changes: 0 additions & 42 deletions polkadot/node/core/pvf/prepare-worker/src/executor_intf.rs

This file was deleted.

Loading
Loading