Skip to content

perf(sources): whale-aware census spill bypasses pickle round trip#3237

Merged
Sinity merged 1 commit into
masterfrom
perf/sources/whale-aware-census-spill
Jul 21, 2026
Merged

perf(sources): whale-aware census spill bypasses pickle round trip#3237
Sinity merged 1 commit into
masterfrom
perf/sources/whale-aware-census-spill

Conversation

@Sinity

@Sinity Sinity commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

The historical-backfill census spill (polylogue/sources/revision_backfill.py:_ParsedSessionSpill) now holds oversized ("whale") parsed session trees resident in memory during census+replay instead of always pickling them to a scratch sqlite file and reloading via pickle.loads — the dominant cost on whale-bearing rebuild pages per the v42 walk receipts.

Problem

2026-07-21 v42 walk receipts: on whale-bearing rebuild pages, the spill_load stage timer dominates wall time — 598.2s of a 1440.2s page (41%), plus 236.6s/523.1s, 228.1s/607.5s, 139.6s/454.6s on other pages.

_ParsedSessionSpill pickles decoded ParsedSession trees to a scratch sqlite file during census and reloads them (for_raw()) one authority cohort at a time during replay. The spill's own in-RAM hot cache (_decoded) is tree-byte budgeted (256 MiB–2 GiB, adaptive from effective_physical_memory_bytes() // 16, see polylogue/pipeline/parsed_tree_size.py) — any single tree above that budget was unconditionally excluded from the hot cache, so every reload of a genuinely oversized session (e.g. a 442 MB Codex rollout) paid a full pickle.loads, or — once the sqlite spill's own payload-byte budget was also exhausted by other raws sharing the page — a full re-parse from raw bytes on every access.

Solution

Added a second, wider "whale ceiling" budget derived from the same effective_physical_memory_bytes() machinery already used for the hot cache (physical // 4, capped at 8 GiB, floored at whatever the hot-cache budget resolved to). A tree too large for the hot cache but within the whale ceiling now bypasses the sqlite spill entirely — held resident as a plain Python object in a new _whales dict, no pickle.dumps at census, no pickle.loads on replay.

Correctness-over-speed fallbacks (per AC):

  • A tree exceeding even the whale ceiling still falls back to the unmodified sqlite-spill path (_retain_whale returns False, add() proceeds exactly as before).
  • If accepting a new whale would exceed the whale budget, the oldest resident whale is evicted (FIFO) — but rather than being dropped, it is degraded into the sqlite spill (extracted _spill_to_sqlite helper, shared with the normal path) so it still avoids a full reparse-from-source on its next access.

Rejected levers:

  • Stream-reparse-on-reload: not pursued. The measured baseline cost is the pickle round trip itself (see Verification), not "we lack the bytes" — residency removes exactly that cost, and reparsing from raw bytes is the existing worse fallback this change avoids, not a competitive alternative.
  • Cheaper spill serialization (pickle protocol/compression): not pursued. Bypassing serialization entirely for the dominant case (the whale) already removes the cost that lever would only shrink; no new dependency was needed or added.

No schema change (spill layer only, matches AC). Single-writer invariant unchanged (the spill remains process-local, discarded on __exit__). Replay output equivalence is asserted directly by tests (see below), not merely implied by "we didn't touch the classification logic."

Verification

  • devtools test tests/unit/sources/test_revision_backfill.py tests/benchmarks/test_whale_census_spill_bench.py tests/infra/revision_backfill_benchmark.py45 passed, 1 pre-existing failure (test_backfill_resumes_after_replay_batch_crash_discards_whole_batch_cleanly, messages_fts_identity UNIQUE constraint) — confirmed failing identically on unmodified origin/master via a git stash/pop before/after comparison; unrelated to this change, not touched by this PR.
  • mypy --strict on all four touched/added files: Success: no issues found in 4 source files.
  • devtools verify --quick: exit 0 (ran automatically via pre-push hook too).
  • Benchmark receipts (tests/benchmarks/test_whale_census_spill_bench.py, harness committed and reproducible — see module docstring for how it neutralizes only the new _retain_whale branch to reconstruct a faithful "baseline" from the same unmodified code path, avoiding a second hand-written implementation): 40 MB synthetic whale + 40 small raws, 2 reload passes:
    • baseline (pre-lever pickle round trip): 81.9ms
    • lever (resident dict lookup, same 2 accesses): ~5µs
    • ≈16,000x on the reload step at this scaled-down fixture size (chosen to keep CI wall time bounded per the bead's own "scale down proportionally" guidance — the ratio, not the absolute byte count, is the measurement).
    • Direct pickle.loads scaling check: 6 MB → 2.1ms, 60 MB → 21.7ms (~0.36ms/MB), which extrapolates to ~160ms for a single reload of a production-scale 442 MB whale — multiplied by however many for_raw() calls a page's classification loops make against that raw_id, consistent with the multi-hundred-second spill_load receipts.
    • Anti-vacuity: reverting _retain_whale to always return False (exactly what the "baseline" run does) reproduces the pre-lever cost — this is not a test-only validator, it exercises the real _ParsedSessionSpill.add()/for_raw() production code paths with only that one branch neutralized.

AC matrix (polylogue-odm1)

AC Status
Profile first with synthetic whale fixture, measure current + candidate levers Satisfied — tests/benchmarks/test_whale_census_spill_bench.py + direct pickle-scaling probes
Size-partitioned spill: threshold derived from existing tree-byte budget machinery, not a new magic number Satisfied — whale ceiling reuses effective_physical_memory_bytes(), same divisor family as the existing hot-cache budget, floored at it; rationale documented in the class docstring
Whales held resident counted against overall memory budget; fallback to spilling if would blow budget Satisfied — independent _whale_tree_bytes accounting bounded by _whale_budget; _retain_whale returns False (falls through to sqlite spill) when a tree exceeds the ceiling; eviction degrades into sqlite spill rather than dropping
Stream-reparse-on-reload lever Evaluated, rejected — see Solution section; measured cost is the pickle round trip, not a missing-bytes gap
Cheaper serialization lever Evaluated, rejected — residency already removes the serialization cost entirely for the dominant case; no new dependency
Preserve single-writer invariant Satisfied — no change to writer topology
Replay outputs byte-identical (counts/hashes/authority decisions); benchmark asserts equivalence, not just speed Satisfied — test_whale_bypasses_pickle_round_trip_on_reload asserts message-text equality baseline vs lever; test_backfill_replays_whale_bearing_page_byte_identical_to_content proves the full backfill_historical_revision_evidence pipeline replays every raw (whale + small) to correct session/message counts with the lever active
No schema changes Satisfied — spill layer only
Record measured delta on polylogue-623q when it lands Deferred to a follow-up note on polylogue-623q referencing this PR's receipts (not done as part of this PR; no bead state was mutated by this branch)

Ref polylogue-odm1

Problem: v42 walk receipts (2026-07-21) show spill_load dominating
whale-bearing rebuild pages -- 598.2s of a 1440.2s page (41%), plus
236.6s/523.1s, 228.1s/607.5s, 139.6s/454.6s on other pages. The census
spill (_ParsedSessionSpill) pickles decoded ParsedSession trees to a
scratch sqlite file during census and reloads them during replay. A
tree too large for the spill's own small hot-cache budget (256MiB-2GiB,
tree-byte denominated) never qualified for that in-RAM cache, so every
for_raw() reload paid a full pickle.loads of the whale, or -- once the
sqlite spill's own payload-byte budget was also exhausted by other raws
in the page -- a full re-parse from raw bytes.

Solution: a second, wider "whale ceiling" budget (physical RAM / 4,
capped 8 GiB, floored at the existing hot-cache budget) lets an
oversized tree bypass the sqlite spill entirely and stay resident as a
plain Python object -- no pickle.dumps at census, no pickle.loads on
replay. Falls back to the existing sqlite-spill path whenever a single
tree exceeds even the whale ceiling (correctness over speed), and a
whale evicted from the resident tier by a larger one is degraded into
the sqlite spill rather than dropped, so it still avoids a full reparse
on next access. No schema change; single-writer and replay-output
invariants unchanged.

Rejected levers: stream-reparse-on-reload was not pursued because the
measured baseline cost is the pickle round trip itself, not a "we don't
have this session's bytes" gap -- residency directly removes the cost
that's actually being paid, and reparsing from raw bytes is the
existing *worse* fallback, not an alternative worth trading for.
Cheaper serialization (pickle protocol/compression) was not pursued
because bypassing serialization entirely for the dominant case (the
whale itself) already removes the cost that lever would only shrink.

Modules touched: polylogue/sources/revision_backfill.py
(_ParsedSessionSpill: _retain_whale, _spill_to_sqlite extraction,
for_raw whale lookup), tests/infra/revision_backfill_benchmark.py
(build_whale_bearing_corpus + WHALE_BEARING_SHAPE), new
tests/benchmarks/test_whale_census_spill_bench.py, and correctness
tests in tests/unit/sources/test_revision_backfill.py.

Verification:
- devtools test tests/unit/sources/test_revision_backfill.py
  tests/benchmarks/test_whale_census_spill_bench.py
  tests/infra/revision_backfill_benchmark.py -> 45 passed, 1
  pre-existing failure unrelated to this change
  (test_backfill_resumes_after_replay_batch_crash_discards_whole_batch_cleanly,
  confirmed failing identically on unmodified origin/master via
  `git stash` before/after comparison -- messages_fts_identity
  UNIQUE constraint, not touched by this PR).
- mypy --strict on all four touched/added files: no issues found.
- devtools verify --quick: exit 0.
- Benchmark receipts (tests/benchmarks/test_whale_census_spill_bench.py,
  40MB synthetic whale + 40 small raws, 2 reload passes):
  baseline (pre-lever pickle round trip) = 81.9ms, lever (resident
  dict lookup) = ~5us for the same 2 accesses -- roughly 16,000x on
  the reload step alone at this scaled-down fixture size; production
  442MB whales measured directly at 21.7ms per pickle.loads per MB-60
  (linear extrapolation puts a single reload near 160ms, multiplied
  by however many for_raw() calls a page's classification loops make
  against that raw_id).

Ref polylogue-odm1
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Sinity, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f0b8a277-1157-49a0-9cde-54e47351c059

📥 Commits

Reviewing files that changed from the base of the PR and between 2390728 and c0f8bd8.

📒 Files selected for processing (4)
  • polylogue/sources/revision_backfill.py
  • tests/benchmarks/test_whale_census_spill_bench.py
  • tests/infra/revision_backfill_benchmark.py
  • tests/unit/sources/test_revision_backfill.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/sources/whale-aware-census-spill

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Sinity

Sinity commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Coordinator stand-in review (CodeRabbit rate-limited): read the full 698-line diff.

  • Lever choice is right: the profiling showed the cost IS the pickle round trip (~0.36ms/MB scaling probe → ~160ms/reload extrapolated to the 442MB production whale × per-page for_raw multiplicity, consistent with the 598s spill_load receipt), so residency beats stream-reparse and cheaper serialization — the rejected-levers reasoning holds.
  • Bounded correctly: whale tier budgeted independently via the same effective_physical_memory_bytes machinery (physical/4 capped 8GiB, floored at the hot budget); over-ceiling trees fall through to the UNCHANGED sqlite path, and multi-whale eviction degrades to the sqlite spill rather than dropping (never worse than baseline — proven by test).
  • Aliasing note (accepted): for_raw on a resident whale returns the same objects across calls, but the pre-existing _decoded hot cache already has exactly this semantic for small sessions, so no new behavior class is introduced.
  • Output equivalence asserted both at the spill-class level (baseline-vs-lever content equality with the baseline reproduced by neutralizing only the new branch) and end-to-end through backfill_historical_revision_evidence.
  • 45 tests green, 1 pre-existing failure confirmed on unmodified master (the feat(storage): add messages_fts_identity ledger for rowid-reuse detection #3235 identity-ledger xdist issue — already routed to the in-flight FTS lane), mypy strict clean.

Merging on green quick-gate.

@Sinity
Sinity merged commit a220c63 into master Jul 21, 2026
2 of 3 checks passed
@Sinity
Sinity deleted the perf/sources/whale-aware-census-spill branch July 21, 2026 12:53
Sinity added a commit that referenced this pull request Jul 21, 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