Skip to content

Commit 6deae81

Browse files
authored
fix(bb): size Pippenger MSM arena for the non-GLV mid-band (#24249)
## Problem UltraHonk `bb prove` intermittently aborts in CI with: ``` libc++abi: terminating due to uncaught exception of type std::runtime_error: Assertion failed: (aligned_local + bytes <= bound) Left : 4123456 Right : 3699643 ``` The assertion is `MsmArena::bump_alloc`'s `BB_ASSERT_LTE(aligned_local + bytes, bound)` in `scalar_multiplication_fast.cpp`. It is **not** OOM — the per-MSM scratch arena was sized smaller than the live allocator consumes. ## Root cause `compute_arena_bytes_for_msm` sizes the arena from the conservative full-bit schedule (`NUM_BITS = FULL_NUM_BITS = 254` for non-GLV). The live path (`pippenger_round_parallel`) then shrinks the budget to `effective_num_bits` = the observed max scalar MSB, which can make `choose_window_bits` pick a *smaller* `window_bits` ⇒ *more* windows ⇒ a larger Zone S than the full-bit pre-size. The sizer already has a defensive sweep that takes the max arena footprint across every bit budget, but it was gated `if (use_glv || n_input >= 2^17)`. That left the **non-GLV mid-band** (`GLV_SMALL_N_THRESHOLD < n_input < 2^17`, i.e. `8192 < n < 131072` natively) sized for the full-bit schedule only. `simple_shield`'s commitment MSM is ~28,696 points — non-GLV, squarely in that gap — so a witness whose scalars drive a smaller `effective_num_bits` overflows the arena. It's intermittent because it needs both a runner core-count that yields the heavier schedule (this run reported 8 threads) and witness data whose max scalar MSB lands in the trigger band (~224–250). Both vary per run/PR, so it isn't tied to any particular PR. ## Fix Make the defensive bit-budget sweep unconditional. `use_glv || n_input > GLV_SMALL_N_THRESHOLD` is true for every MSM that reaches this point, so the sweep now covers GLV and all non-GLV sizes. It only ever grows the arena when a shrunk-bit layout genuinely needs more, so the common full-bit-dominated case stays tight. Arena sizing is scratch memory only — no effect on VKs or proof output. ## Test Added `MidBandArenaSizerCoversAllEffectiveNumBits`, which drives the module's own arena-fit simulator across the non-GLV mid-band × every `effective_num_bits` at 8 threads. It fails on the pre-fix sizer (under-counts at n=28,696 for `effective_num_bits` 224–248, among others) and passes with the fix. Full `ScalarMultiplication*` suite (110 tests, incl. real-MSM correctness and the small-scalar band test) passes.
2 parents d0239d1 + fa17227 commit 6deae81

3 files changed

Lines changed: 58 additions & 16 deletions

File tree

barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/pippenger_arena_layout.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ struct VariableWindowSchedule {
5959
size_t num_windows = 0;
6060
std::array<uint8_t, VAR_WINDOW_MAX_WINDOWS> window_bits_per_window{}; // window_bits_w for each w
6161
std::array<uint16_t, VAR_WINDOW_MAX_WINDOWS> bit_base{}; // B_w = Σ_{k<w} c_k, B_0 = 0
62-
std::array<uint16_t, VAR_WINDOW_MAX_WINDOWS> num_buckets{}; // 2^(window_bits_w - 1) + 1
62+
// 2^(window_bits_w - 1) + 1. uint32_t: window_bits = 17 gives 65537, one past uint16_t, and the
63+
// cost model can pick window_bits up to 18 for very large MSMs (n approaching the 2^26 SRS cap).
64+
std::array<uint32_t, VAR_WINDOW_MAX_WINDOWS> num_buckets{};
6365
};
6466

6567
// Per-chunk recursive-affine bucket-reduce output (Stage 6b output cell).
@@ -130,7 +132,7 @@ inline VariableWindowSchedule build_var_window_schedule(size_t num_bits, size_t
130132
const size_t window_bits_w = std::min<size_t>(window_bits, bits_remaining);
131133
sched.bit_base[w] = static_cast<uint16_t>(bit_offset);
132134
sched.window_bits_per_window[w] = static_cast<uint8_t>(window_bits_w);
133-
sched.num_buckets[w] = static_cast<uint16_t>((size_t{ 1 } << (window_bits_w - 1)) + 1);
135+
sched.num_buckets[w] = static_cast<uint32_t>((size_t{ 1 } << (window_bits_w - 1)) + 1);
134136
bit_offset += window_bits_w;
135137
bits_remaining -= window_bits_w;
136138
++w;

barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication.test.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,6 +1738,36 @@ TEST(ScalarMultiplicationArenaTest, ArenaLayoutFitsAcrossDispatchSpace)
17381738
bb::set_parallel_for_concurrency(saved_threads);
17391739
}
17401740

1741+
// Non-GLV mid-band (GLV_SMALL_N_THRESHOLD < n < 2^17) arena-sizing coverage. The live allocator
1742+
// shrinks the window-bit budget to the observed scalar msb, which can pick a heavier schedule than
1743+
// the full-bit pre-sizer; `compute_arena_bytes_for_msm` must upper-bound the arena across every
1744+
// effective_num_bits. Regression for the `bb prove` abort `Assertion failed: (aligned_local + bytes
1745+
// <= bound)` on UltraHonk simple_shield (~28,696-point commitment MSM, 8 threads): that n sits in
1746+
// this band, which the dispatch sweep (probing only 8193 and 262144) and the small-scalar band test
1747+
// (n <= 16384) both miss.
1748+
TEST(ScalarMultiplicationArenaTest, MidBandArenaSizerCoversAllEffectiveNumBits)
1749+
{
1750+
const size_t saved_threads = bb::get_num_cpus();
1751+
bb::set_parallel_for_concurrency(8);
1752+
1753+
bool found_undersize = false;
1754+
for (const size_t n :
1755+
{ size_t{ 28696 }, size_t{ 8193 }, size_t{ 1 } << 14, size_t{ 1 } << 15, size_t{ 1 } << 16 }) {
1756+
for (size_t bits = 1; bits <= 254; ++bits) {
1757+
if (!pippenger_bn254_arena_layout_fits_for_test(n,
1758+
/*external_glv_provided=*/false,
1759+
/*dedup_active=*/false,
1760+
bits)) {
1761+
info("UNDERSIZE: n=", n, " effective_num_bits=", bits, " threads=8");
1762+
found_undersize = true;
1763+
}
1764+
}
1765+
}
1766+
1767+
bb::set_parallel_for_concurrency(saved_threads);
1768+
EXPECT_FALSE(found_undersize) << "arena sizer under-counts in the non-GLV mid-band";
1769+
}
1770+
17411771
// ======================= Test Wrappers =======================
17421772

17431773
TYPED_TEST(ScalarMultiplicationTest, PippengerLowMemory)

barretenberg/cpp/src/barretenberg/ecc/scalar_multiplication/scalar_multiplication_fast.cpp

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,13 +1211,10 @@ size_t compute_arena_bytes_for_msm(size_t n_input, bool external_glv_provided, b
12111211
const size_t dedup_bytes = dedup_active ? ((size_t{ 4 } * n) + (size_t{ sizeof(typename Curve::AffineElement) } *
12121212
round_parallel_detail::DEDUP_MAX_CLUSTERS))
12131213
: size_t{ 0 };
1214-
auto arena_bytes_for_window_layout = [&](size_t bit_budget) {
1215-
const size_t wb = round_parallel_detail::choose_window_bits(n, bit_budget, n_input, num_logical_threads_for_c);
1214+
auto arena_bytes_for_window_layout = [&](size_t bit_budget, size_t wb) {
12161215
const auto layout_sched = round_parallel_detail::build_var_window_schedule(bit_budget, wb);
1217-
size_t B_eff_layout = (size_t{ 1 } << (wb - 1)) + 1;
1218-
for (size_t w = 0; w < layout_sched.num_windows; ++w) {
1219-
B_eff_layout = std::max(B_eff_layout, static_cast<size_t>(layout_sched.num_buckets[w]));
1220-
}
1216+
// Uniform schedule: the widest window's bucket count is the per-window cap.
1217+
const size_t B_eff_layout = (size_t{ 1 } << (wb - 1)) + 1;
12211218
const size_t dense_stride_layout = round_parallel_detail::compute_dense_stride(B_eff_layout, num_threads);
12221219
const size_t per_window_bytes_layout = round_parallel_detail::compute_per_window_bytes<Curve>(
12231220
num_threads, B_eff_layout, n, dense_stride_layout, worker_total_for_budget);
@@ -1238,14 +1235,27 @@ size_t compute_arena_bytes_for_msm(size_t n_input, bool external_glv_provided, b
12381235
// first-touch cost regardless of how much of the arena the small MSM_fast actually uses.
12391236
size_t arena_bytes = fixed_overhead + (windows_per_batch * per_window_bytes) + 32768 + dedup_bytes;
12401237

1241-
// The live pipeline shrinks NUM_BITS to the observed max scalar bit before choosing
1242-
// window_bits. GLV MSMs and large non-GLV MSMs can therefore select a different
1243-
// schedule/zone layout than the full-bit pre-sizer. Keep the common Chonk wire/IPA
1244-
// non-GLV sizes on the original tight path.
1245-
if (use_glv || n_input >= (size_t{ 1 } << 17)) {
1246-
for (size_t bit_budget = 1; bit_budget <= NUM_BITS; ++bit_budget) {
1247-
arena_bytes = std::max(arena_bytes, arena_bytes_for_window_layout(bit_budget));
1248-
}
1238+
// The live pipeline chooses window_bits from the *effective* (nonzero) scalar count and the
1239+
// observed bit budget after Phase 1: c = choose_window_bits(n_active, effective_num_bits) with
1240+
// n_active <= n and effective_num_bits <= NUM_BITS. Fewer active points => smaller c => more
1241+
// windows => a larger arena (most sharply once fixed_overhead has eaten the batch budget and
1242+
// every window runs in a single batch). Size for the worst reachable c so the bound holds for
1243+
// any scalar density, with no extra scalar scan.
1244+
//
1245+
// For a fixed c, bit_budget = NUM_BITS maximizes the window count (effective_num_bits <=
1246+
// NUM_BITS) and 2^(c-1)+1 caps B_eff, so arena_bytes_for_window_layout(NUM_BITS, c) dominates
1247+
// every live (effective_num_bits, c) layout. The reachable c span is [2, c_max]: choose is
1248+
// non-decreasing in the point count (n_active <= n bounds it above), but the ceil() in the round
1249+
// count makes it non-monotonic in the bit budget by ±1, so c_max is the max over bit budgets,
1250+
// not simply choose(n, NUM_BITS).
1251+
size_t c_max_reachable = window_bits;
1252+
for (size_t bit_budget = 1; bit_budget <= NUM_BITS; ++bit_budget) {
1253+
c_max_reachable = std::max(c_max_reachable,
1254+
static_cast<size_t>(round_parallel_detail::choose_window_bits(
1255+
n, bit_budget, n_input, num_logical_threads_for_c)));
1256+
}
1257+
for (size_t wb = 2; wb <= c_max_reachable; ++wb) {
1258+
arena_bytes = std::max(arena_bytes, arena_bytes_for_window_layout(NUM_BITS, wb));
12491259
}
12501260
return arena_bytes;
12511261
}

0 commit comments

Comments
 (0)