Skip to content

Prototype: spill Buf and HeapBuf into private regions#103

Merged
typeless merged 2 commits into
mainfrom
proto/buf-region-spill
Jul 18, 2026
Merged

Prototype: spill Buf and HeapBuf into private regions#103
typeless merged 2 commits into
mainfrom
proto/buf-region-spill

Conversation

@typeless

Copy link
Copy Markdown
Owner

What

Replaces the bump-region spill path in Buf and HeapBuf with a private Region owned by each buffer (container-as-allocator taken to its conclusion for the string builders):

  • Buf::grow past the 4096-byte inline buffer now commits into an owned vm reservation (16 MiB initial, replaced with a doubled reservation on exhaustion — no abort at any size).
  • HeapBuf becomes a thin owner of a Region plus a size; its backing store is the region directly.
  • Region gains a reserved() accessor to support the replacement-growth path.
  • The obstack calls (bump_try_extend/bump_try_pop) disappear from both types; the destructor releases the whole reservation unconditionally.

Why

A case analysis of all ~137 Buf/HeapBuf sites showed the buffers that actually spill (command/expression expansion, read_file contents, process-output accumulators) are exactly the ones whose lifetime brackets other bump allocations — so LIFO reclaim structurally cannot fire for them, and every regrowth after an interleaved allocation orphans a block. Owning the spill as a region makes growth in-place (base never moves) and reclamation unconditional, as a structural property rather than a lifetime coincidence.

Measurements (gcc-example parse, 1852 commands, perf -r 5)

metric main this PR delta
cycles 329.7M ±0.6% 336.6M ±0.5% +2.1%
instructions 444.5M 454.9M +2.3%
cache-misses 313K 289K −7.7%
peak RSS 46.5 MB 46.0 MB −0.5 MB
bump gross 20.5 MB 13.4 MB −7.1 MB
bump net 11.64 MB 11.34 MB −0.3 MB

Honest read: the parse benchmark moves 7.1 MB of gross churn off the bump but recovers only ~0.5 MB RSS — on this workload the obstack was already reclaiming ~95% of spill bytes, and parse never exercises the two structurally-worst cases (JobSlot process-output buffers don't run without executing commands; read_file only sees 24 small Tupfiles). The cost is +2% cycles from mmap traffic. The value delivered is the structural guarantee (no spill can strand or churn regardless of interleaving) and full memory return on buffer death — the measured parse-time win is small.

Tests

Four new RED-first tests: spills consume zero bump bytes (Buf + HeapBuf), spilled pointer stays stable while the bump top is taken by another allocation, and growth past the 16 MiB reservation preserves contents off-bump. Plus a pin for HeapBuf move semantics (a stale comment claimed it was non-movable). Full suite: 60271 assertions in 525 cases, green. make iwyu clean; clang-tidy has no diagnostics in the touched production files.

🤖 Generated with Claude Code

https://claude.ai/code/session_01U5KUoGDhJEZYoiK9CkeXtz

@typeless

Copy link
Copy Markdown
Owner Author

Full-build benchmark (gcc example, 3529 commands from scratch, putup-only counters via perf stat -i, 3 runs main / 2 runs spill):

metric main spill verdict
cycles 27.92–28.76 G 28.05–28.07 G inside main's noise — parse's +2.1% does not reproduce
instructions 47.44 G 47.71 G +0.6% deterministic
peak RSS (VmHWM) 1015.7–1032.4 MB 1011.0–1022.1 MB ranges overlap — no measurable delta
bump gross 112.5 MB 90.9 MB −21.6 MB deterministic
bump net 62.9 MB 61.6 MB −1.3 MB

The RSS comparison is swamped by an unrelated discovery the benchmark surfaced: putup's build-time peak RSS is ~1 GB, of which ~845 MB is a single arena mapping produced by Arena32::append_extend having no at-tail fast path — every one-element append to a node's edge slice copies the whole slice to the arena tail (O(k²) bytes for a k-edge node; Index::add_edge hit it 160k times in a no-op build, ~500 MB of pure memcpy). Until that is fixed, allocator-level RSS deltas of ~20 MB are unmeasurable at the process level. Filed as follow-up work.

🤖 Generated with Claude Code

@typeless
typeless force-pushed the proto/buf-region-spill branch from 3a7ece7 to 5561828 Compare July 18, 2026 02:10
@typeless

Copy link
Copy Markdown
Owner Author

Re-benchmark on top of the arena fix (#104 merged; branch rebased onto main c9bde20, 520 cases green). With the ~1 GB arena noise gone, allocator-level deltas are now measurable:

workload metric main this PR delta
parse peak RSS 18.3 MB 17.9 MB −0.4 MB
parse cycles 318.2 M 324.4 M +1.9%
dry-run peak RSS 51.0 MB 50.5 MB −0.5 MB
dry-run cycles 830 M 848 M +2.2%
full build peak RSS (VmHWM) 86.2 MB 85.2 MB −0.9 MB
full build cycles 27.56 G 27.65 G +0.3% (noise)
full build bump gross / net 112.9 / 62.6 MB 91.1 / 61.7 MB −21.7 / −1.0 MB

(Instructions +0.6% on builds, +2.3–2.7% on parse/dry-run, deterministic. The spill full-build wall clock was polluted by concurrent local work — cycles are the trustworthy column there.)

Updated verdict: the picture sharpens but doesn't change — ~2% cycles on parse-shaped workloads and neutral on builds buys the structural no-strand guarantee and −21.7 MB of deterministic gross churn, with ≤1 MB of realized RSS on today's workloads. This is now purely a judgment call on whether the insurance is worth the small constant cost.

🤖 Generated with Claude Code

@typeless

Copy link
Copy Markdown
Owner Author

Re-benchmark on top of the arena fix (#104, now on main; branch rebased onto c9bde2021, full suite green at 62,537 assertions). With the ~1 GB arena churn gone, allocator-level deltas are finally measurable at the process level. Baseline = main with arena fix, both binaries built from the same tree.

workload metric main this PR delta
parse (perf -r 5) peak RSS 18.35 MB 17.91 MB −0.44 MB
cycles 318.2 M ±0.4% 324.4 M ±0.6% +1.9%
dry-run -n (perf -r 5) peak RSS 50.99 MB 50.50 MB −0.49 MB
cycles 830 M ±0.2% 848 M ±0.3% +2.2%
full build (3529 cmds) peak RSS (VmHWM) 86.15 MB 85.24 MB −0.91 MB
cycles 27.56 G 27.65 G +0.3% (noise)
cache-misses 25.4 M 22.6 M −11%
bump gross / net 112.9 / 62.6 MB 91.1 / 61.7 MB −21.7 / −1.0 MB

(Side note: parse peak RSS dropped 46.5 → 18.3 MB from #104 alone — the parse-phase DAG construction hit the same quadratic.)

Updated verdict: the RSS win is real but small (0.4–0.9 MB, ~1–2.5%) at a cost of ~+2% cycles on graph-heavy operations (neutral on full builds, where fork/hash dominates). The deterministic effects — 21.7 MB of gross churn off the bump, and the structural guarantee that spills can never strand regardless of allocation interleaving — are unchanged. This is now purely a robustness-vs-constant-cost judgment call, with honest numbers on both sides.

🤖 Generated with Claude Code

A spilled Buf's lifetime routinely brackets other bump allocations
(interleaved Vec growth, nested Bufs, operator new), so its blocks can
neither extend in place nor be reclaimed at destruction; HeapBuf file
contents and process output strand structurally, pinned under
everything their consumer allocates. Owning the spill as a per-buffer
vm reservation makes growth in-place and reclamation unconditional:
the region is released whole when the buffer dies. Reservations start
at 16 MiB and are replaced with a doubled reservation on exhaustion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5KUoGDhJEZYoiK9CkeXtz
@typeless
typeless force-pushed the proto/buf-region-spill branch from 5561828 to 0d35a52 Compare July 18, 2026 04:51
@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown

PR metrics

Performance (gcc example, Linux)

Workload Instructions CPU time Page faults D1 miss LL miss Wall Peak RSS
parse n/a 0.47 s 14.1 k (+15.6%) 1.1% 0.1% 0.464 s 32.8 MB (-0.3MB)
dry-run n/a 0.7 s 18.6 k (+12.7%) 1.6% 0.1% 0.712 s 49.8 MB (-1.7MB)

Deterministic signals: page faults, peak RSS, and the cachegrind D1/LL miss rates (simulated cache, exact across runs). CPU time is user+sys from time(1); instructions read n/a on GitHub-hosted runners (virtualized, no PMU).

Test coverage (lines)

Overall Median file Min file Max file
85.1% (+0.1pp) 94.3% (+2.3pp) 0.0% include/pup/core/metrics.hpp 100.0% include/pup/core/arena.hpp

93 files · 13697/16104 lines covered

Deltas vs main@052d1a8bd.

Updated for 2804c30

@typeless

Copy link
Copy Markdown
Owner Author

Follow-up pushed: HeapBuf is deleted (c6c992b, −418/+96).

The audit behind it: HeapBuf's move-by-value was load-bearing at exactly one site (read_file's return). Every other use — process capture on both platforms, JobSlot accumulators, win32 wide-conversion scratch — accumulates, interns into the pool, and discards: Buf's job. And unlike HeapBuf, Buf's 4 KiB inline buffer absorbs small files and small command outputs without any region round-trip, which removes the per-small-file mmap/munmap cost this PR previously added to parse.

Changes:

  • read_file(path, Buf& out) -> Result<void> — follows the established convention (return value = immutable, output param = mutable); Buf gained resize() (RED-first, zero-fill extend semantics carried over from HeapBuf).
  • All former HeapBuf sites now use Buf; the nullable-data_ observer checks (data() ? data() : "") go with it — Buf's data pointer is structurally never null.
  • Docs: DESIGN.md/STYLE.md primitive tables updated (string building is now a two-way split: Buf builds, StringId stores).

Verification: Linux suite 62,523 assertions in 512 cases green; Windows cross-build clean and 60,953 assertions in 287 cases green under Wine (~[e2e]~[shell]); clang-tidy clean on touched files; iwyu clean.

HeapBuf duplicated Buf's whole API for one load-bearing difference:
being returnable by value from read_file. Every other use site
(process capture, JobSlot accumulators, win32 conversion scratch)
accumulates, interns, and discards - exactly Buf's job, and Buf's
4 KiB inline buffer now absorbs the common small case instead of
paying a region round-trip per call.

read_file flips to the output-parameter convention (return value =
immutable, output param = mutable): read_file(path, Buf&) ->
Result<void>. Buf gains resize() so the read loop can write straight
into it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5KUoGDhJEZYoiK9CkeXtz
@typeless
typeless force-pushed the proto/buf-region-spill branch from 1d8b0fb to 2804c30 Compare July 18, 2026 05:56
@typeless
typeless merged commit dabf463 into main Jul 18, 2026
11 checks passed
@typeless
typeless deleted the proto/buf-region-spill branch July 18, 2026 06:14
@typeless typeless mentioned this pull request Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant