diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index 4fb0863d70..d6c4f37658 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -173,6 +173,39 @@ jobs: - name: cargo test --doc --workspace --all-features run: cargo test --doc --workspace --all-features + # The `jemalloc-allocator` feature is off by default, so the default build + # never compiles it. Without an explicit gate it rots silently: a + # `tikv-jemallocator` bump, a `#[global_allocator]` typo, or a `cfg` mistake + # would break the opt-in allocator path with nothing failing. This job + # compiles+links the node with the feature (build gate) and runs the + # activation test proving jemalloc is the *live* global allocator, not merely + # linked - the failure mode a plain `cargo check` cannot see. + jemalloc-feature: + name: jemalloc-allocator feature gate + needs: [trusted-pr, changes] + if: github.event_name != 'pull_request' || needs.changes.outputs.rust == 'true' + runs-on: [self-hosted, fireactions-turbo-8] + environment: + name: ${{ github.event_name == 'workflow_dispatch' && 'sccache-reader' || 'sccache-writer' }} + deployment: false + env: + SKIP_WASM_BUILD: 1 + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/rust-setup + with: + cache-key: jemalloc-feature + sccache-credential-mode: auto + sccache-writer-access-key-id: ${{ secrets.SCCACHE_R2_WRITE_ACCESS_KEY_ID }} + sccache-writer-secret-access-key: ${{ secrets.SCCACHE_R2_WRITE_SECRET_ACCESS_KEY }} + - name: Build gate (node compiles + links the feature) + run: cargo check -p node-subtensor --features jemalloc-allocator --all-targets + # No test-name filter: a filtered `cargo test` exits 0 if the test is ever + # renamed (0 run), so the gate would pass silently. Running the node's + # full test target with the feature guarantees the activation test runs. + - name: Activation test (jemalloc is the live global allocator) + run: cargo test -p node-subtensor --features jemalloc-allocator + # The browser seam: bittensor-core without its `host` feature must keep # compiling for wasm32 (the future wasm-bindgen/TS binding target). This # check is what keeps "browser-compatible" an invariant instead of an diff --git a/node/Cargo.toml b/node/Cargo.toml index 333fc4a093..da20b81641 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -132,6 +132,19 @@ pallet-subtensor-swap-rpc = { workspace = true, features = ["std"] } pallet-subtensor-swap-runtime-api = { workspace = true, features = ["std"] } subtensor-macros.workspace = true +# Optional global allocator (off by default). Archive nodes serving sustained RPC load show +# unbounded anonymous-memory growth (~3-4 GB/h) that fits the glibc-malloc arena-fragmentation +# profile across the node's hundreds of threads; linking jemalloc as the global allocator (as the +# polkadot binary does) is the proposed mitigation (issue #2724). Gated behind a feature so the +# default release binary is unchanged until an operator opts in. +tikv-jemallocator = { version = "0.5", optional = true, default-features = false } +# Companion to `tikv-jemallocator`, used by the `jemalloc-allocator` feature's +# activation test to read allocator stats over `mallctl`. It already resolves +# transitively and shares the single compiled `tikv-jemalloc-sys` with +# `tikv-jemallocator` (see Cargo.lock), so enabling it adds no second copy of +# jemalloc - only this small ctl FFI shim. +tikv-jemalloc-ctl = { version = "0.5", optional = true } + [build-dependencies] substrate-build-script-utils.workspace = true @@ -145,6 +158,9 @@ rocksdb = [ "sc-cli/rocksdb", ] default = ["rocksdb", "sql", "txpool"] +# Use tikv-jemallocator as the global allocator in the release binary (off by default). +# See the `tikv-jemallocator` dependency note and issue #2724. +jemalloc-allocator = ["dep:tikv-jemallocator", "dep:tikv-jemalloc-ctl"] fast-runtime = [ "node-subtensor-runtime/fast-runtime", "subtensor-runtime-common/fast-runtime", diff --git a/node/src/main.rs b/node/src/main.rs index a6aa15038f..cd23935b24 100644 --- a/node/src/main.rs +++ b/node/src/main.rs @@ -1,6 +1,14 @@ //! Substrate Node Subtensor CLI library. #![warn(missing_docs)] +// Use jemalloc as the global allocator when the `jemalloc-allocator` feature is enabled (off by +// default). Archive nodes serving sustained RPC load exhibit unbounded anonymous-memory growth that +// matches the glibc-malloc arena-fragmentation profile across the node's hundreds of threads; +// jemalloc (the allocator the polkadot binary ships with) avoids it. See issue #2724. +#[cfg(all(unix, feature = "jemalloc-allocator"))] +#[global_allocator] +static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; + #[cfg(feature = "runtime-benchmarks")] mod benchmarking; mod chain_spec; @@ -18,3 +26,71 @@ mod service; fn main() -> sc_cli::Result<()> { command::run() } + +// Regression guard for the optional `jemalloc-allocator` feature. +// +// The feature is off by default, so the default build never compiles it, and a +// `tikv-jemallocator` bump, a `#[global_allocator]` typo, or a `cfg` mistake can +// leave the binary *compiling* while jemalloc is no longer the active global +// allocator (glibc silently stays in charge). A compile check cannot catch that: +// only bytes routed *through* jemalloc are observable, so this test drives the +// global allocator with a large live allocation and asserts jemalloc tracked it. +// If the `#[global_allocator]`/`cfg` wiring is broken the Vec goes to glibc and +// the delta stays at zero - well below the threshold - and the test fails. +// +// The delta is read from jemalloc's *thread-local* "bytes ever allocated by this +// thread" counter, not the process-global `stats.allocated`. `cargo test` runs the +// node's tests in parallel inside one process, and the global stat is perturbed by +// every other test's allocations and frees, so a before/after delta around it is +// racy: a concurrent free can drop it below threshold (false failure) and a +// concurrent allocation can lift it (false pass). The thread-local counter only +// ever counts bytes the *calling* thread routes through the global allocator, so +// it is immune to concurrent tests; it is also monotonic (a free cannot decrease +// it), so the only way it stays flat across a large live allocation is if that +// allocation went to glibc because jemalloc is linked but not active. +#[cfg(all(test, unix, feature = "jemalloc-allocator"))] +mod jemalloc_allocator_feature { + use tikv_jemalloc_ctl::thread; + + // A live allocation routed through the *global* allocator. Large enough to + // dwarf any bookkeeping the stat reads do themselves. + const PAYLOAD: usize = 16 * 1024 * 1024; + + // The workspace denies `expect_used`/`unwrap_used`, and `tikv_jemalloc_ctl`'s + // Error is a transparent wrapper that does not implement std::error::Error, + // so the Result cannot be propagated with `?` either. Resolve the stat handle + // by hand instead. + fn allocated_pointer() -> thread::ThreadLocal { + match thread::allocatedp::read() { + Ok(pointer) => pointer, + Err(error) => panic!("read thread.allocatedp: {error:?}"), + } + } + + #[test] + fn jemalloc_is_the_active_global_allocator() { + // `read()` returns a thread-local pointer to the cumulative byte count + // for *this* thread; `get()` dereferences it (safe: the raw-pointer read + // is encapsulated in `tikv_jemalloc_ctl`). Thread stats are live, so - + // unlike `stats::allocated` - no epoch refresh is needed before a read. + let pointer = allocated_pointer(); + let before = pointer.get(); + + // Held across the second read so the allocator cannot reclaim it first. + let hold = vec![0u8; PAYLOAD]; + + let after = pointer.get(); + + assert!( + after.saturating_sub(before) >= PAYLOAD as u64, + "jemalloc did not track a {}-byte global-allocator allocation \ + (before={}, after={}): it is linked but not the active global \ + allocator, so the jemalloc-allocator feature wiring is broken", + PAYLOAD, + before, + after, + ); + + drop(hold); + } +}