Skip to content

Commit adcf759

Browse files
committed
Merge main and drop the runtime jemalloc knob
`main` brought #996, which compiles jemalloc's never-purge policy into the binary as `_rjem_malloc_conf`, read before `main` is entered. This branch carried #995's `keep_large_buffers_warm`, which set the same policy at runtime through `mallctl` on the oversize arena. Two generations of one idea by one author, and the conflict is the whole of both: the compiled form wins. Taking it drops what only the runtime form needed — its call in the prove-and-retire path, the `Cargo.toml` note explaining why `tikv-jemalloc-ctl` carries no `stats` in terms of that function, and the `log` dependency #995 declared for its warnings, which nothing else in the CLI uses. `tikv-jemalloc-ctl` itself stays: the heap tracker reads `stats.allocated` through it. Worth knowing rather than buried: #995 kept the knob off `verify`, `execute` and `--help` on the grounds that disabling decay for the life of the process "would otherwise change the allocator under every baseline measured with this binary". #996 applies it binary-wide and offers `_RJEM_MALLOC_CONF` in the environment as the way back to the default policy. Anyone taking memory numbers with this binary is measuring the never-purge policy unless they set that variable, and it has to be that spelling. `main` also brings `prover/tests/jemalloc_conf.rs`, which reads both options back out of jemalloc — it pins the policy this merge keeps. Verified: `cli` builds with and without `jemalloc-stats`, and `make lint` is clean across its four arms.
2 parents 95bd62b + ffc4ac1 commit adcf759

5 files changed

Lines changed: 147 additions & 81 deletions

File tree

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/cli/Cargo.toml

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,11 @@ tempfile = "3"
1616
tikv-jemallocator = "0.6"
1717
# No `stats` by default: that feature propagates to tikv-jemalloc-sys and builds
1818
# jemalloc with `--enable-stats`, i.e. counters on the malloc fast path, for every
19-
# binary. `keep_large_buffers_warm` needs only `raw`/`mallctl`; the heap tracker is
20-
# what needs the counters, and it is behind `jemalloc-stats`.
19+
# binary. The heap tracker is what needs the counters, and it is behind
20+
# `jemalloc-stats`.
2121
tikv-jemalloc-ctl = { version = "0.6" }
2222
tikv-jemalloc-sys = "0.6"
2323
env_logger = "0.11"
24-
# Used by `keep_large_buffers_warm`, whose body is Linux-only — so a macOS build
25-
# will not catch its absence.
26-
log = "0.4"
2724

2825
[features]
2926
jemalloc-stats = ["tikv-jemalloc-ctl/stats"]

bin/cli/src/main.rs

Lines changed: 48 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -11,78 +11,54 @@ use clap::{Parser, Subcommand, ValueHint};
1111
#[global_allocator]
1212
static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
1313

14-
/// jemalloc serves allocations of 8 MiB and up from one shared arena and purges each of
15-
/// them the moment it is freed, whatever the decay says, unless decay is disabled for that
16-
/// arena (`extent_may_force_decay`). A prover that allocates and drops one trace-sized
17-
/// buffer after another then refaults and re-zeroes the same pages for every chunk.
18-
/// Disable the arena's decay and purge it on our own clock instead: hot buffers are
19-
/// reused across threads, cold ones still go back to the OS.
20-
///
21-
/// The index is `opt.narenas`: jemalloc 5 reserves the slot immediately after the
22-
/// automatic arenas for the oversize arena (`arena_init_huge`, `huge_arena_ind =
23-
/// narenas_total_get()`), and `opt.oversize_threshold` defaults to the same 8 MiB.
24-
///
25-
/// Linux only: elsewhere jemalloc is built without background threads and the decay
26-
/// mallctl traps.
27-
///
28-
/// Called only from the paths that allocate trace-sized buffers, not from `main`:
29-
/// it disables an arena's decay for the life of the process and leaves a purge
30-
/// thread behind, which is not something `cli verify` or `--help` should pay, and
31-
/// it would otherwise change the allocator under every baseline measured with
32-
/// this binary.
33-
fn keep_large_buffers_warm() {
34-
#[cfg(target_os = "linux")]
35-
{
36-
use std::ffi::CString;
37-
use std::ptr::null_mut;
38-
use std::time::Duration;
39-
use tikv_jemalloc_ctl::raw;
40-
41-
const PURGE_EVERY: Duration = Duration::from_secs(10);
42-
43-
// The arena only exists after the first large allocation.
44-
std::hint::black_box(vec![0u8; 16 << 20]);
45-
// SAFETY: `opt.narenas` is `unsigned`, the decay knob is `ssize_t`, and `purge`
46-
// takes no value.
47-
unsafe {
48-
let huge_arena = match raw::read::<u32>(b"opt.narenas\0") {
49-
Ok(n) => n,
50-
Err(e) => {
51-
// Not fatal, but the run is then indistinguishable from one
52-
// where the knob worked — which is exactly what makes a
53-
// memory measurement unreadable. Say so.
54-
log::warn!(
55-
"keep_large_buffers_warm: cannot read opt.narenas ({e}); \
56-
the oversize arena keeps jemalloc's default decay"
57-
);
58-
return;
59-
}
60-
};
61-
let decay = format!("arena.{huge_arena}.dirty_decay_ms\0");
62-
if let Err(e) = raw::write(decay.as_bytes(), -1i64) {
63-
log::warn!(
64-
"keep_large_buffers_warm: cannot disable decay on arena \
65-
{huge_arena} ({e}); the oversize arena keeps jemalloc's default"
66-
);
67-
return;
68-
}
69-
log::debug!("keep_large_buffers_warm: decay disabled on arena {huge_arena}");
70-
let purge = CString::new(format!("arena.{huge_arena}.purge")).unwrap();
71-
std::thread::spawn(move || {
72-
loop {
73-
std::thread::sleep(PURGE_EVERY);
74-
tikv_jemalloc_sys::mallctl(
75-
purge.as_ptr(),
76-
null_mut(),
77-
null_mut(),
78-
null_mut(),
79-
0,
80-
);
81-
}
82-
});
83-
}
84-
}
85-
}
14+
// jemalloc, never purging.
15+
//
16+
// The allocator itself is unchanged: jemalloc is here because the platform
17+
// allocator keeps freed arena chunks resident, and on the recursion campaign's
18+
// branch the same proves read up to 13 GiB higher under glibc. What this sets
19+
// is jemalloc's *decay* timers, which hand freed pages back to the OS. The
20+
// prover allocates and frees multi-hundred-MiB host buffers continuously, so
21+
// those pages come straight back as minor faults on the worker threads.
22+
//
23+
// Measured on an RTX 5090 box on the recursion campaign's branches, ABBA in
24+
// each, every arm at one commit and one set of knobs:
25+
// * the WHIR prover (keccak, `whir/lfm` @ 64393da9) — 39.69-39.88 s a block
26+
// with this setting against 43.94-44.07 s without, the default costing
27+
// +13 M minor faults and +15 s of system time per run on this arm;
28+
// * the per-table STARK tree (0e4f4610) — 187 / 163 / 166 / 171 s, both
29+
// never-purge arms under both default arms, 5-24 s a block, with the proof
30+
// bytes unmoved (30 identical lines, 0 differing).
31+
// The cost is peak RSS: +1.4 GiB and +2.9-3.3 GiB respectively, a ninth to a
32+
// quarter of the 13 GiB the allocator choice itself is worth — which is why the
33+
// lever is the decay setting and not the allocator. `background_thread:true` recovers
34+
// none of it: the cost is the re-touch, not the `madvise` call.
35+
//
36+
// This binary's own pipeline has not been measured under the setting; the
37+
// numbers above are from the campaign's branches, where the prover's
38+
// allocation pattern is the same.
39+
//
40+
// `_RJEM_MALLOC_CONF` in the environment still overrides this, which is how a
41+
// measurement arm puts the default policy back. It has to be that spelling:
42+
// `tikv-jemalloc-sys` builds with `--with-jemalloc-prefix=_rjem_` under default
43+
// features, and jemalloc then reads one env name chosen at configure time
44+
// (`jemalloc.c`, `obtain_malloc_conf` source 3) — so plain `MALLOC_CONF` is read
45+
// by nothing here and sets an arm to the default policy without saying it did
46+
// not. The file source is prefixed too: `/etc/_rjem_malloc.conf`.
47+
//
48+
// jemalloc reads this symbol as a `const char *` before `main` is entered, so
49+
// the value has to be in the initializer, and the name is the prefixed one
50+
// `tikv-jemalloc-sys` declares (`#[cfg_attr(prefixed, link_name =
51+
// "_rjem_malloc_conf")]`, its `src/lib.rs`). None of that is compiler-checked.
52+
// `prover/tests/jemalloc_conf.rs` reads both options back out of jemalloc, but
53+
// it carries its own copy of this block and reads its own process — it pins the
54+
// pattern, not this export. Deleting the lines below turns nothing red.
55+
const NEVER_PURGE: &[u8] = b"dirty_decay_ms:-1,muzzy_decay_ms:-1\0";
56+
57+
#[allow(non_upper_case_globals)]
58+
#[unsafe(export_name = "_rjem_malloc_conf")]
59+
pub static malloc_conf: Option<&'static core::ffi::c_char> =
60+
Some(unsafe { &*(NEVER_PURGE.as_ptr() as *const core::ffi::c_char) });
61+
8662
use executor::vm::instruction::decoding::Instruction;
8763
use executor::vm::instruction::execution::{Accelerator, SyscallNumbers};
8864
use executor::{elf::Elf, flamegraph::FlamegraphGenerator, vm::execution::Executor};
@@ -1414,9 +1390,6 @@ fn cmd_trace_build(
14141390
}
14151391
};
14161392
let outcome = if prove_and_retire {
1417-
// The walk allocates and drops one trace-sized buffer after another,
1418-
// which is the pattern this works around.
1419-
keep_large_buffers_warm();
14201393
run_approach_1(
14211394
&elf,
14221395
&elf_data,

prover/tests/calibration.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,22 @@ use tikv_jemalloc_ctl::{epoch, stats};
2121
#[global_allocator]
2222
static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
2323

24+
// ...with the shipped binary's purge policy, so this binary is the production
25+
// allocator *configuration* and not just the production allocator. The reason
26+
// and the numbers are at `bin/cli/src/main.rs`. `prover/tests/jemalloc_conf.rs`
27+
// asserts that this export pattern is read, but it does so against its own copy
28+
// in its own process — nothing checks the copy below.
29+
//
30+
// It moves nothing this file asserts — `stats::allocated` is live bytes, which
31+
// the decay timers do not touch; a resident-memory assertion added here later
32+
// would read the wrong configuration without it.
33+
const NEVER_PURGE: &[u8] = b"dirty_decay_ms:-1,muzzy_decay_ms:-1\0";
34+
35+
#[allow(non_upper_case_globals)]
36+
#[unsafe(export_name = "_rjem_malloc_conf")]
37+
pub static malloc_conf: Option<&'static core::ffi::c_char> =
38+
Some(unsafe { &*(NEVER_PURGE.as_ptr() as *const core::ffi::c_char) });
39+
2440
fn allocated_bytes() -> usize {
2541
epoch::advance().ok();
2642
stats::allocated::read().unwrap_or(0)

prover/tests/jemalloc_conf.rs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
//! jemalloc's purge policy is compiled into the binary — checked by reading it
2+
//! back out of the allocator serving this process.
3+
//!
4+
//! `bin/cli/src/main.rs` and `calibration.rs` each export
5+
//! `_rjem_malloc_conf = "dirty_decay_ms:-1,muzzy_decay_ms:-1"` beside their
6+
//! `#[global_allocator]`, so the never-purge policy travels in the binary
7+
//! rather than in a launcher's environment. Nothing about that export is
8+
//! checked by the compiler: a misspelled symbol, a wrong value type, or a
9+
//! jemalloc built without the `_rjem_` prefix each leave a binary that
10+
//! compiles, links, runs — and quietly purges.
11+
//!
12+
//! This is its own test binary because the check needs a jemalloc process of
13+
//! its own: the prover's lib tests run under the platform allocator, where a
14+
//! `mallctl` read would say nothing, and `calibration.rs` is behind
15+
//! `disk-spill` and pays for a full proof. What it pins is the export pattern —
16+
//! symbol, type, initializer, edition spelling — in the copy below, which is
17+
//! byte-identical to the two production sites but not mechanically tied to
18+
//! them: delete either of those and this still passes. It is a self-test of the
19+
//! pattern, not a regression guard on the two sites that ship it. That the
20+
//! shipped `cli` binary carries the symbol is a link-time property, read with
21+
//! `nm` rather than asserted here.
22+
23+
use tikv_jemalloc_ctl::raw;
24+
25+
#[global_allocator]
26+
static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
27+
28+
/// The same string the two production sites export, character for character.
29+
const NEVER_PURGE: &[u8] = b"dirty_decay_ms:-1,muzzy_decay_ms:-1\0";
30+
31+
#[allow(non_upper_case_globals)]
32+
#[unsafe(export_name = "_rjem_malloc_conf")]
33+
pub static malloc_conf: Option<&'static core::ffi::c_char> =
34+
Some(unsafe { &*(NEVER_PURGE.as_ptr() as *const core::ffi::c_char) });
35+
36+
/// jemalloc's default `opt.dirty_decay_ms`, quoted in the failure message so a
37+
/// red test says which value it found and where that value comes from.
38+
const DEFAULT_DIRTY_DECAY_MS: isize = 10_000;
39+
40+
#[test]
41+
fn jemalloc_never_purge_is_compiled_in() {
42+
// `_RJEM_MALLOC_CONF` sets these same options from the environment, and
43+
// benchmark runs do set it; with it set, reading `-1` back would say nothing
44+
// about the compiled-in export, so refuse to run rather than pass for the
45+
// wrong reason. Plain `MALLOC_CONF` is inert in this prefixed build — guarded
46+
// anyway, so that a future unprefixed build does not silently pass here.
47+
// Not covered: `/etc/_rjem_malloc.conf`, the one remaining source that could
48+
// set `opt.*` from outside this binary.
49+
for var in ["_RJEM_MALLOC_CONF", "MALLOC_CONF"] {
50+
assert!(
51+
std::env::var_os(var).is_none(),
52+
"{var} is set in this process's environment. jemalloc reads \
53+
`_RJEM_MALLOC_CONF` (this build is prefixed), which sets `opt.*` on \
54+
its own, so this test could not tell the compiled-in export from the \
55+
environment; unset it and re-run."
56+
);
57+
}
58+
59+
// `opt.dirty_decay_ms` and `opt.muzzy_decay_ms` are jemalloc `ssize_t`s.
60+
// `raw::read` asserts the mallctl's width equals `size_of::<T>()`, so a
61+
// wrong Rust width fails here rather than reading a truncated value.
62+
let dirty: isize =
63+
unsafe { raw::read(b"opt.dirty_decay_ms\0") }.expect("opt.dirty_decay_ms is readable");
64+
let muzzy: isize =
65+
unsafe { raw::read(b"opt.muzzy_decay_ms\0") }.expect("opt.muzzy_decay_ms is readable");
66+
67+
assert_eq!(
68+
dirty, -1,
69+
"opt.dirty_decay_ms is {dirty}, not -1 (jemalloc's default is \
70+
{DEFAULT_DIRTY_DECAY_MS}): the `_rjem_malloc_conf` export beside this \
71+
file's `#[global_allocator]` is missing, misspelled, or was not read, \
72+
and a binary built this way returns dirty pages to the OS on a timer"
73+
);
74+
assert_eq!(
75+
muzzy, -1,
76+
"opt.muzzy_decay_ms is {muzzy}, not -1 (jemalloc's default is 0): the \
77+
`_rjem_malloc_conf` export beside this file's `#[global_allocator]` is \
78+
missing, misspelled, or was not read, and a binary built this way \
79+
unmaps muzzy pages immediately"
80+
);
81+
}

0 commit comments

Comments
 (0)