Skip to content

Commit 0d5abe2

Browse files
authored
Review fixes for prove-and-retire (#994) (#995)
* Reject a malformed out-of-domain block instead of panicking `verify_batched`'s fold seed indexes both out-of-domain blocks at the `width`/`height` the proof advertises, but `ood_blocks_well_formed` — the guard that pins those dimensions to the AIR and calls `dimensions_consistent()` — did not run until 92 lines later. A proof whose advertised dimensions disagree with its data length therefore panicked in `Table::get_row`'s unchecked slice rather than being rejected. That is the exact gap the guard's own doc comment says it exists to close, and the ordinary verifier keeps the ordering by running it inside the round-1 loop. Hoist the three shape checks into the pre-pass that already validates each table's domain, before any of them is read. The division by `trace_length` is safe there: the same loop rejects zero first. No transcript byte moves — the checks touch no transcript. `prover::batched_verifier::replay` had the same pre-guard read through `Table::columns`; it is test-only, but a patch that fixed only the STARK half would leave a reader thinking the family was covered. Direction is robustness, not soundness: nothing wrong is accepted, the verifier aborts instead of returning false. It is unreachable from bytes today because `BatchedProof` has no derives — which is also why it is the cheapest moment to pay for it, since serializing that format is the point. Also drop `Replay::iotas`, documented as the per-group query indices and always empty, and say plainly that `replay` is a test oracle: it takes the prover's word on the precomputed root and never checks `fold_order` is a permutation, both of which `verify` does. `a_tampered_batched_proof_is_rejected` grows five arms: a group's FRI layer root (the one commitment batching relocated from per-table to per-group), a main root, a composition root, an out-of-domain value, and a block whose advertised width lies. The last one panics at `table.rs:362` without this change and is rejected with it. * Reattach eight doc comments to the items they describe Each of these inserted a new item between an existing doc comment and the item it documented, with no blank line, so rustdoc merged the two blocks and the original item lost its docs: prover.rs table_parallelism -> MainRoots prover.rs plain -> known_roots verifier.rs replay_rounds_* -> replay_rounds_2_and_3 decode.rs update_multiplicities -> add_multiplicities trace_builder.rs cpu32_chip_op -> WalkLeftover trace_builder.rs build_initial_image-> runtime_page_ranges trace_builder.rs touched_memory_cells -> op_count trace_builder.rs collect_epoch -> walk_and_emit_chunks Three of the adopted sentences were actively wrong about their new owner: `known_roots` was labelled "for a plain (non-preprocessed) table" when it takes `precomputed: Option<Commitment>` and serves both; `add_multiplicities` was described in terms of a `lookups` parameter it does not have; and `WalkLeftover`'s public rustdoc opened by describing an ALU dispatch helper. `replay_rounds_2_and_3` is renamed `replay_rounds_2_to_4`: its body still carries an explicit `Round 4` section sampling gamma and the DEEP coefficients, so the orphaned sentence ("rounds 2, 3 and 4") was the accurate one and the new name was not. `bitwise_histogram` carried two stacked doc blocks, the first saying LT, MUL, DVRM, SHIFT, the accelerators and PAGE "are not here yet" and the second, directly below, saying the histogram is complete. The first is left over from an earlier state; `finalize` folds all of them in. * Say what the walk actually holds, and drop a stale table count Two claims about residency were wrong in the same way. `pass.rs`'s header said what the walk holds is one table plus the residents "no matter how long the run is", and the design doc listed LT among the tables handed over "as soon as a chunk fills". LT is not: it, MUL, DVRM and SHIFT are deliberately absent from `CHUNKED_KINDS` because later derivations keep appending to them, so their chunk boundaries are not knowable until the run ends — `walk_and_emit_chunks`'s own doc comment says so two paragraphs below the sentence that contradicted it. Their op lists, the `retired_*` rows a closing chunk converts its ops into, and the walk's BITWISE lookups are all held whole, so that term is O(cycles). It is a small term — compact routed intermediates against trace rows, a low single-digit percentage of the measured peak — and closing it would move LT's chunk boundaries and cost the byte-identical-roots property that makes the per-table variant a drop-in. So this changes no code: it makes the documents say what the code does, and lists the gap in §10 with the observation that BITWISE's half is the cheap one, since a histogram is commutative and could be folded per segment without moving any root. Also in the design doc: - the `A1_TABLE_PARALLELISM` row quoted a sweep ("flat between 12 and 24; 24 costs 6 GB") that does not match the one recorded on `pass::table_parallelism` (no k=12 or k=24 rows; the step is 16 -> 32 for 5.0 GB); - `A1_INFLIGHT` and `LAMBDA_STREAM_LDE` were missing from a table that claims to list every knob, and the second changes this approach's own memory profile; - `--features hash-metrics` is from #987, which is not on this branch, so the verify-hash column cannot be reproduced here — say so rather than give a build command that fails; - "the spec's Open optimization ... was not kept" described code that ships: what was dropped is holding whole Merkle trees between passes, while leaf-dropping is `drop_leaves`/`retire_leaves` behind `LAMBDA_STREAM_LDE`; - §6's header omitted the blowup and the knob settings the numbers were taken at. And five comments said the ethrex block has 227 tables where the doc says 245. Rather than guess which run is stale, they now say "once per table" and the like: none of them needed the number. * Stop the CLI changing the allocator for every command Three things, all outside the prove-and-retire path. `keep_large_buffers_warm()` ran as the first statement of `main()`, so every subcommand — `prove`, `verify`, `execute`, `--help` — allocated 16 MiB, disabled dirty decay on the oversize arena for the life of the process, and left a 10-second purge thread behind. Disabling decay retains RSS that `auto_storage::available_ram_bytes()` does not model, and it is the sort of change that quietly moves every memory number taken with this binary. Call it from the prove-and-retire path, which is the one that allocates and drops trace-sized buffers in a loop. Both of its mallctl failure paths returned silently, and `env_logger::init()` ran on the next line, so nothing could have been logged even if it had tried. A run where the knob did not land was indistinguishable from one where it did. They now warn. The doc comment also records why `opt.narenas` is the right index — jemalloc 5 reserves the slot after the automatic arenas for the oversize arena (`arena_init_huge`), whose threshold defaults to the same 8 MiB the comment names — since a count used as an index invites a second look. `tikv-jemalloc-ctl` had become a hard dependency carrying `features = ["stats"]`, and `jemalloc-stats` an empty feature. That propagates to `tikv-jemalloc-sys/stats` and so to `--enable-stats`, which puts counters on the malloc fast path of every CLI build, including ones measuring baselines. `keep_large_buffers_warm` needs only `raw`/`mallctl`, so the dependency stays and `stats` goes back behind `jemalloc-stats`, which is what the heap tracker is gated on anyway. Finally, `--output` with any stage but `logup` walked the whole execution, returned no proof, wrote no file and exited 0 — and with `--through batched` it also forced a verification the user had not asked for, because `--output` is OR'd into the `verify` argument. It now fails before the walk with a message naming the stage. * Make four test assertions able to fail `retire_lde_proof_is_byte_identical` compared a proof against itself under `cuda`: there `retire_leaves` returns `None` unconditionally and `retire_main_lde` is compiled out, so both arms take the resident path. That configuration is not hypothetical — `make test-prover-cuda` runs this suite on the merge queue. It is now `#[cfg(not(feature = "cuda"))]`, and each arm asserts `streaming_retire_lde()` actually returned what it set, so the test fails rather than passes if the flag ever stops taking effect. Its `ENV_LOCK` was a function-local `static` that nothing else could name, and libtest calls each `#[test]` once, so it could never be contended — it guarded nothing, and the SAFETY comment above the `set_var` ("single-threaded section guarded by ENV_LOCK") was false on both clauses. Replaced with what is actually true: this is the only writer in the binary, every reader goes through `std::env`, which serialises readers against writers on its own lock, so the exposure is other tests observing the flag under a plain `cargo test` — their coverage, not memory safety. `cargo nextest`, which CI runs, forks per test. The note names the real fix (its own integration binary, as `prover/tests/gpu_force_downgrade.rs` already does) without doing it here. `checkpoint_tests`' `assert!(full.len() > 100_000)` followed an `assert_eq!(full.len(), N_ADDI + 1)` with `N_ADDI = 100_005` — a tautology. The property it was reaching for is already checked by the `logs.len() < full.len()` assertion further down. `chunk_shape_matches_the_built_chunk` gave ops to LT only, so for the other thirteen kinds both sides collapsed to the 4-row padding floor and only the column width was pinned. The row half was covered, but by one kind — so a divergence in a single generator's padding would be missed. It now also populates MUL (dedup, like LT) and SHIFT (plain, 20 ops over a limit of 8, so its chunks are 8/8/4 and sit above the floor), asserts each fixture exercises what it is there for, and counts populated chunks so the loop cannot silently go back to comparing constants. `prover/src/tests/mod.rs` declared `batched_fri_tests` and `challenge_phase_tests` without the `#[cfg(test)]` every other entry carries; the parent `mod tests` is ungated, so those two were the only ones compiled into a non-test build of the library. * Satisfy the lint gate `cargo fmt --all`, plus a `clone()` on a `Copy` field that the new out-of-domain tamper arm introduced. * Declare the `log` dependency the CLI actually uses `keep_large_buffers_warm`'s warnings are inside `#[cfg(target_os = "linux")]`, so a macOS build never compiles them and my local lint runs said nothing. CI, on Linux, did: `use of unresolved module or unlinked crate log`. `log` was reaching `bin/cli` only as a transitive dependency of `env_logger`, which is not a dependency you may name. Declared, with a note on the file that its only user is Linux-gated. * Build the prover without `parallel` again `make compile-recursion-elfs` compiles `lambda-vm-prover` for the RISC-V guest, where `parallel` is off and there is no rayon. The three new phase modules `use rayon::prelude::*` unconditionally and call `into_par_iter`/`par_iter`, so the recursion guest stopped building: 27 errors, 8 unresolved-`rayon` and 9 missing-method, plus three `E0505`s in `trace_builder`. This is on #994's branch as it stands, not introduced by this PR — the same `cargo check -p lambda-vm-prover --no-default-features` fails identically at `f800e4b0`. It went unnoticed because no CI run has ever touched that branch; this PR is the first, which is how it surfaced. The four `make lint` arms do not catch it either: the workspace-level `--no-default-features` arm still resolves `parallel` through another member's feature unification. Gated with the idiom already used in `trace_builder.rs` — a `#[cfg]` pair around the iterator source, serial arm `into_iter`/`iter`. Where the closure was long enough that duplicating it would be worse than the problem, it is hoisted to a named binding first and both arms map over that, so the body appears once. No behaviour change on any path that runs today: the serial arms exist to compile for the guest, which links the crate for its verifier and never executes these phases. The `E0505`s were the serial arm of the BITWISE collector loop iterating `&collectors` where the parallel arm moves it into `units`, so the closures' borrows of the op lists outlived the point where `CollectedOps` moves those lists. Consumed by value, matching the parallel arm. Verified: `make compile-recursion-elfs` succeeds, all four `make lint` arms and `cargo fmt --check` pass, and the prove-and-retire tests are unchanged at 13/13.
1 parent f800e4b commit 0d5abe2

20 files changed

Lines changed: 505 additions & 219 deletions

‎Cargo.lock‎

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

‎bin/cli/Cargo.toml‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,19 @@ clap = { version = "4.3.10", features = ["derive"] }
1414
rkyv = { version = "0.8.10", default-features = false, features = ["alloc", "bytecheck", "aligned", "pointer_width_64"] }
1515
tempfile = "3"
1616
tikv-jemallocator = "0.6"
17-
tikv-jemalloc-ctl = { version = "0.6", features = ["stats"] }
17+
# No `stats` by default: that feature propagates to tikv-jemalloc-sys and builds
18+
# 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`.
21+
tikv-jemalloc-ctl = { version = "0.6" }
1822
tikv-jemalloc-sys = "0.6"
1923
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"
2027

2128
[features]
22-
jemalloc-stats = []
29+
jemalloc-stats = ["tikv-jemalloc-ctl/stats"]
2330
disk-spill = ["prover/disk-spill"]
2431
instruments = ["prover/instruments", "stark/instruments"]
2532
# GPU profiling build (Nsight): CUDA prover + instruments spans + NVTX ranges.

‎bin/cli/src/main.rs‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,18 @@ static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
1818
/// Disable the arena's decay and purge it on our own clock instead: hot buffers are
1919
/// reused across threads, cold ones still go back to the OS.
2020
///
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+
///
2125
/// Linux only: elsewhere jemalloc is built without background threads and the decay
2226
/// 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.
2333
fn keep_large_buffers_warm() {
2434
#[cfg(target_os = "linux")]
2535
{
@@ -35,13 +45,28 @@ fn keep_large_buffers_warm() {
3545
// SAFETY: `opt.narenas` is `unsigned`, the decay knob is `ssize_t`, and `purge`
3646
// takes no value.
3747
unsafe {
38-
let Ok(huge_arena) = raw::read::<u32>(b"opt.narenas\0") else {
39-
return;
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+
}
4060
};
4161
let decay = format!("arena.{huge_arena}.dirty_decay_ms\0");
42-
if raw::write(decay.as_bytes(), -1i64).is_err() {
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+
);
4367
return;
4468
}
69+
log::debug!("keep_large_buffers_warm: decay disabled on arena {huge_arena}");
4570
let purge = CString::new(format!("arena.{huge_arena}.purge")).unwrap();
4671
std::thread::spawn(move || {
4772
loop {
@@ -356,7 +381,6 @@ enum Stage {
356381
}
357382

358383
fn main() -> ExitCode {
359-
keep_large_buffers_warm();
360384
env_logger::init();
361385
let cli = Cli::parse();
362386

@@ -1258,7 +1282,7 @@ fn report_batched_size(proof: &prover::logup_phase::BatchedProof, tables: usize,
12581282

12591283
/// Where the time went, summed per span label.
12601284
///
1261-
/// The prover's own spans are per table and there are 227 of them, so the raw
1285+
/// The prover's own spans are per table and there are hundreds of them, so the raw
12621286
/// timeline is unreadable; what answers "where is the time" is the total per
12631287
/// label. Sums exceed wall time, because tables run concurrently — the ratios
12641288
/// between labels are the point, not the absolute figures.
@@ -1338,6 +1362,23 @@ fn cmd_trace_build(
13381362
verify: bool,
13391363
output: Option<PathBuf>,
13401364
) -> ExitCode {
1365+
// Only the per-table proof is serializable today, so `--output` with any
1366+
// other stage would walk the whole execution and then write nothing. Say so
1367+
// before spending the walk rather than exiting 0 in silence.
1368+
if output.is_some() && prove_and_retire && through != Stage::Logup {
1369+
eprintln!(
1370+
"--output writes the per-table proof, which only `--through logup` assembles; \
1371+
`--through {}` has no serializable proof yet (see docs/prove_and_retire_design.md \u{00A7}10).",
1372+
match through {
1373+
Stage::Walk => "walk",
1374+
Stage::Commit => "commit",
1375+
Stage::Challenge => "challenge",
1376+
Stage::Batched => "batched",
1377+
Stage::Logup => unreachable!(),
1378+
}
1379+
);
1380+
return ExitCode::FAILURE;
1381+
}
13411382
let elf_data = match std::fs::read(&elf_path) {
13421383
Ok(data) => data,
13431384
Err(e) => {
@@ -1373,6 +1414,9 @@ fn cmd_trace_build(
13731414
}
13741415
};
13751416
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();
13761420
run_approach_1(
13771421
&elf,
13781422
&elf_data,

‎crypto/crypto/src/merkle_tree/merkle.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,15 +330,20 @@ where
330330
leaves_len: usize,
331331
sibling_leaf: B::Node,
332332
) -> Option<Proof<B::Node>> {
333-
if leaves_len <= 1 || pos >= leaves_len {
333+
if leaves_len <= 1 || pos >= leaves_len || self.is_root_only() {
334334
return None;
335335
}
336336
let mut merkle_path = Vec::with_capacity(leaves_len.trailing_zeros() as usize);
337337
merkle_path.push(sibling_leaf);
338338

339339
let mut node = parent_index(pos + leaves_len - 1);
340340
while node != ROOT {
341-
merkle_path.push(self.nodes.get(sibling_index(node))?.clone());
341+
// `node_get`, not `self.nodes` directly: every other read in this
342+
// file goes through it for the disk-spill mmap indirection. The two
343+
// are mutually exclusive today — `drop_leaves` refuses an mmap-backed
344+
// tree — but a direct read would silently yield `None` here if that
345+
// ever stopped holding, and the prover's opening path unwraps this.
346+
merkle_path.push(self.node_get(sibling_index(node))?.clone());
342347
node = parent_index(node);
343348
}
344349
self.create_proof(merkle_path)

‎crypto/stark/src/batched_verifier.rs‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ struct GroupReplay<F: IsFFTField, E: IsField> {
3636
iotas: Vec<usize>,
3737
acc: Vec<FieldElement<E>>,
3838
acc_sym: Vec<FieldElement<E>>,
39+
/// The first table of the group, whose AIR supplied the domain. Kept from
40+
/// the scan that already found it rather than looked up again: the second
41+
/// scan is what forced an `expect` into verifier code.
42+
member: usize,
3943
}
4044

4145
/// Verify the rounds after round 1 of every table and each group's FRI.
@@ -90,12 +94,33 @@ where
9094
let num_queries = first_air.options().fri_number_of_queries;
9195
let grinding_factor = first_air.context().proof_options.grinding_factor;
9296

93-
// Every table's domain is its group's.
94-
for (idx, (table, &g)) in tables.iter().zip(group_of).enumerate() {
97+
// Every table's domain is its group's, and every block and opening has the
98+
// shape its AIR declares.
99+
//
100+
// Both run before the fold seed below reads a single out-of-domain value.
101+
// That seed indexes the blocks at the dimensions the *proof* advertises, so
102+
// a proof whose advertised dimensions disagree with its data length has to
103+
// be rejected here rather than panic there — the rule `ood_blocks_well_formed`
104+
// documents, and the one the ordinary verifier keeps by running it in round 1.
105+
for (idx, ((air, table), &g)) in airs.iter().zip(tables).zip(group_of).enumerate() {
95106
if table.trace_length == 0 || table.trace_length != groups[g].trace_rows {
96107
error!("batched: table {idx} does not live on its group's domain");
97108
return false;
98109
}
110+
let view = StarkProofView::Owned(table);
111+
// `trace_length` is non-zero by the check above, so the division is safe.
112+
if table.composition_poly_parts_ood_evaluation.len()
113+
!= air.composition_poly_degree_bound(table.trace_length) / table.trace_length
114+
|| !V::<Field, FieldExtension, PI>::ood_blocks_well_formed(*air, view)
115+
|| !V::<Field, FieldExtension, PI>::trace_opening_widths_well_formed(
116+
*air,
117+
view,
118+
num_queries,
119+
)
120+
{
121+
error!("batched: table {idx}'s blocks or openings are malformed");
122+
return false;
123+
}
99124
}
100125

101126
// The fold coefficients, from the seed, in the prover's order.
@@ -182,6 +207,7 @@ where
182207
iotas,
183208
acc: vec![FieldElement::zero(); num_queries],
184209
acc_sym: vec![FieldElement::zero(); num_queries],
210+
member,
185211
});
186212
}
187213

@@ -199,27 +225,17 @@ where
199225
if let Some(ref bpi) = table.bus_public_inputs {
200226
fork.append_field_element(&bpi.table_contribution);
201227
}
228+
// Shapes were pinned to the AIR in the pre-pass above, before the fold
229+
// seed read any of this table's blocks.
202230
let domain = new_verifier_domain(*air, table.trace_length);
203-
if table.composition_poly_parts_ood_evaluation.len()
204-
!= air.composition_poly_degree_bound(table.trace_length) / table.trace_length
205-
|| !V::<Field, FieldExtension, PI>::ood_blocks_well_formed(*air, view)
206-
|| !V::<Field, FieldExtension, PI>::trace_opening_widths_well_formed(
207-
*air,
208-
view,
209-
num_queries,
210-
)
211-
{
212-
error!("batched: table {idx}'s blocks or openings are malformed");
213-
return false;
214-
}
215231
let layout = V::<Field, FieldExtension, PI>::ood_layout(*air);
216232
let RoundsChallenges {
217233
z,
218234
boundary_coeffs,
219235
transition_coeffs,
220236
trace_term_coeffs,
221237
gammas,
222-
} = V::<Field, FieldExtension, PI>::replay_rounds_2_and_3(
238+
} = V::<Field, FieldExtension, PI>::replay_rounds_2_to_4(
223239
*air,
224240
view,
225241
&public_inputs[idx],
@@ -291,10 +307,7 @@ where
291307

292308
// Each group's FRI, from the folded first layer down to the final polynomial.
293309
for (g, (group, replay)) in groups.iter().zip(replays.iter()).enumerate() {
294-
let member = group_of
295-
.iter()
296-
.position(|&h| h == g)
297-
.expect("checked above");
310+
let member = replay.member;
298311
let fri = group.fri;
299312
let synthetic = StarkProof::<Field, FieldExtension, PI> {
300313
trace_length: group.trace_rows,

‎crypto/stark/src/prover.rs‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,12 @@ impl<F: IsField> TableCommit<F>
131131
where
132132
FieldElement<F>: AsBytes,
133133
{
134-
/// Build a `TableCommit` for a plain (non-preprocessed) table.
135134
/// Roots without a tree, for a pass that will not open against it. The
136135
/// tree is the expensive half; a caller that already knows the roots and
137136
/// only needs them to travel should not pay for one.
137+
///
138+
/// Serves preprocessed and plain tables alike — `precomputed` is `Some`
139+
/// exactly when the AIR is preprocessed.
138140
fn known_roots(root: Commitment, precomputed: Option<Commitment>) -> Self {
139141
Self {
140142
tree: Arc::new(BatchedMerkleTree::from_root(root)),
@@ -146,6 +148,7 @@ where
146148
}
147149
}
148150

151+
/// Build a `TableCommit` for a plain (non-preprocessed) table.
149152
fn plain(#[allow(unused_mut)] mut tree: BatchedMerkleTree<F>, root: Commitment) -> Self {
150153
let leaves_dropped = Self::retire_leaves(&mut tree);
151154
Self {
@@ -646,26 +649,6 @@ fn host_cores() -> usize {
646649
.unwrap_or(4)
647650
}
648651

649-
/// Number of tables `multi_prove` proves concurrently, out of `num_airs` of
650-
/// them.
651-
///
652-
/// Defaults: **every table** under `cuda`, `num_cores / 3` on CPU builds
653-
/// (benchmarked optimal on both M3 Pro and EPYC 9454P — every table there is
654-
/// pure host work, so `k` genuinely competes for cores). Both arms are
655-
/// overridden by the `TABLE_PARALLELISM` env var, and the result is clamped to
656-
/// `1..=num_airs`. Without the `parallel` feature this is 1 and the env var is
657-
/// ignored.
658-
///
659-
/// # Why the `cuda` arm has no core term
660-
///
661-
/// Measured over 881 runs on two RTX 5090 boxes (sweep record linked from
662-
/// PR #911): the work `k` divides is device- and workload-bound — invariant to
663-
/// host core count over an 8× range — so `available_parallelism()` is the
664-
/// wrong quantity to scale `k` by. `k` is not a thread count; it counts
665-
/// concurrent drivers whose per-table work all runs on the one global rayon
666-
/// pool. Worst case against the best measured `k`: `num_airs` +1.6 % (inside
667-
/// noise), the old `cores*2/3` +13.0 %. Bounding concurrency is memory
668-
/// admission's job (`VramGate`), not this count's.
669652
/// A table's Round 1 roots, in the order Fiat-Shamir absorbs them.
670653
///
671654
/// A plain table contributes one root. A preprocessed one contributes two: its
@@ -708,10 +691,6 @@ pub struct TableDeep<FieldExtension: IsField> {
708691
pub trace_rows: usize,
709692
/// The DEEP composition codeword, `lde_size` long.
710693
pub deep: Vec<FieldElement<FieldExtension>>,
711-
/// Where the table sits in the AIR order, carried so the batch can say
712-
/// which group each table ended up in once the codewords are sorted by
713-
/// domain rather than by position.
714-
pub air_index: usize,
715694
/// What the batch's coefficient is drawn from: this table's public round-3
716695
/// data, in the order a transcript absorbs it.
717696
///
@@ -826,6 +805,29 @@ const RETIRE_OVERRIDE_OFF: u8 = 1;
826805
const RETIRE_OVERRIDE_ON: u8 = 2;
827806
static RETIRE_LDE_OVERRIDE: AtomicU8 = AtomicU8::new(RETIRE_OVERRIDE_UNSET);
828807

808+
/// Number of tables `multi_prove` proves concurrently, out of `num_airs` of
809+
/// them.
810+
///
811+
/// Defaults: **every table** under `cuda`, `num_cores / 3` on CPU builds
812+
/// (benchmarked optimal on both M3 Pro and EPYC 9454P — every table there is
813+
/// pure host work, so `k` genuinely competes for cores). Both arms are
814+
/// overridden by the `TABLE_PARALLELISM` env var, and the result is clamped to
815+
/// `1..=num_airs`. Without the `parallel` feature this is 1 and the env var is
816+
/// ignored.
817+
///
818+
/// Not to be confused with `prover::pass::table_parallelism`, which sizes the
819+
/// prove-and-retire walk's batches and reads `A1_TABLE_PARALLELISM`.
820+
///
821+
/// # Why the `cuda` arm has no core term
822+
///
823+
/// Measured over 881 runs on two RTX 5090 boxes (sweep record linked from
824+
/// PR #911): the work `k` divides is device- and workload-bound — invariant to
825+
/// host core count over an 8× range — so `available_parallelism()` is the
826+
/// wrong quantity to scale `k` by. `k` is not a thread count; it counts
827+
/// concurrent drivers whose per-table work all runs on the one global rayon
828+
/// pool. Worst case against the best measured `k`: `num_airs` +1.6 % (inside
829+
/// noise), the old `cores*2/3` +13.0 %. Bounding concurrency is memory
830+
/// admission's job (`VramGate`), not this count's.
829831
pub fn table_parallelism(num_airs: usize) -> usize {
830832
#[cfg(feature = "parallel")]
831833
{
@@ -2102,7 +2104,7 @@ pub trait IsStarkProver<
21022104
///
21032105
/// The codeword is kept rather than the LDEs it came from. That is the
21042106
/// whole reason this split is affordable: on the ethrex block the trace and
2105-
/// composition LDEs of all 227 tables are tens of gigabytes, while their
2107+
/// composition LDEs of every table are tens of gigabytes, while their
21062108
/// DEEP codewords together are about 6.5 GB — one extension element per row
21072109
/// instead of every column. Holding them is what saves walking the
21082110
/// execution again just to recompute them once the coefficient is known.
@@ -2180,7 +2182,6 @@ pub trait IsStarkProver<
21802182
lde_size: domain.interpolation_domain_size * domain.blowup_factor,
21812183
trace_rows: domain.interpolation_domain_size,
21822184
deep,
2183-
air_index: usize::MAX,
21842185
bus_contribution: round_1_result
21852186
.bus_public_inputs
21862187
.as_ref()
@@ -2262,7 +2263,7 @@ pub trait IsStarkProver<
22622263
/// The accumulator is the batch polynomial the spec describes. A member is
22632264
/// added and dropped, so what is held is one codeword per distinct domain
22642265
/// rather than one per table — which on the ethrex block is 13 instead of
2265-
/// 227, and about a gigabyte instead of eight and a half.
2266+
/// one per table, and about a gigabyte instead of eight and a half.
22662267
fn accumulate(
22672268
acc: &mut Vec<FieldElement<FieldExtension>>,
22682269
coefficient: &FieldElement<FieldExtension>,

0 commit comments

Comments
 (0)