perf(daemon): bound parse-stage cache by estimated tree bytes#3209
Conversation
Problem: DaemonParseStage's inflight-bytes admission budget and RawParsePrefetchCache's own admission gate both account raw PAYLOAD bytes, but parsed ParsedSession trees inflate resident memory well beyond payload size (Pydantic model instances, per-block dicts, object overhead). Two earlyoom kills (19.3G and 20.2G RSS peaks, 2026-07-20) happened on a whale-dense page because the cache retained a whole 2000-raw page of parsed trees regardless of the payload-bytes clamp -- clamping inflight bytes did nothing since the pressure came from trees already resident post-parse, not parses in flight. Solution: add estimate_parsed_tree_bytes(), a cheap single-pass structural estimator (sum of text/content field lengths + per-node overhead constant, NOT a recursive sys.getsizeof/pympler-style deep walk) calibrated against a manual deep-object-graph measurement on synthetic sessions (see the comment above the estimator constants in parse_prefetch.py). DaemonParseStage tracks a second budget -- estimated tree bytes, adaptive 1/8 of RAM clamped [256MiB, 4GiB], POLYLOGUE_DAEMON_PARSE_STAGE_MAX_CACHED_TREE_BYTES override -- as a side ledger keyed by raw_id (RawParsePrefetchCache itself is a shared type consumed directly by bulk_rebuild.py, so it stays untouched rather than subclassed). A tree whose estimate exceeds the whole budget is never admitted at all (falls through to the existing writer-held reparse path); admitted trees are evicted largest-first (ties to oldest) via the raw cache's own pop() once the running total exceeds budget, with lazy reconciliation for entries the writer-held pass already consumed out from under the ledger. Verification: - devtools test tests/unit/daemon/test_parse_prefetch.py -> 12 passed - .venv/bin/python -m mypy --strict polylogue/daemon/parse_prefetch.py tests/unit/daemon/test_parse_prefetch.py -> Success: no issues found - devtools verify --quick -> all 16 steps ok, exit 0 Ref polylogue-xb4i Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
CodeRabbit rate-limited (green check is the limit notice, zero inline findings posted). Coordinator review stand-in: read the full diff — calibrated two-term estimator (measured fit documented, both constants rounded up so misestimation biases toward eviction/reparse, never OOM), separate side-ledger keeps RawParsePrefetchCache's shared type untouched for bulk_rebuild consumers, whale-never-retained + largest-first eviction with lazy reconciliation, env override wins. 12 tests green, mypy --strict clean, verify --quick 16/16. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f05b2bc7ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for evicted_id in evicted: | ||
| self.cache.pop(evicted_id) |
There was a problem hiding this comment.
For a large successful warm page, these cache pops do not release the evicted trees: every completed Future remains a key in the local futures dictionary, and each future retains its result tuple (including sessions) until warm_raw_ids() returns. Consequently a whale-dense page can still accumulate the parsed trees for the entire page in memory even though cached_tree_bytes_total is under budget, preserving the peak-RSS/earlyoom failure this change is intended to prevent; completed futures/results need to be discarded as they are processed.
Useful? React with 👍 / 👎.
| physical = _physical_memory_bytes() | ||
| if physical is None: | ||
| return _MIN_MAX_CACHED_TREE_BYTES | ||
| return max(_MIN_MAX_CACHED_TREE_BYTES, min(_MAX_MAX_CACHED_TREE_BYTES, physical // 8)) |
There was a problem hiding this comment.
When the daemon runs in a memory-limited container or systemd cgroup, os.sysconf reports host physical RAM rather than the process's memory.max, so this default can select a 4 GiB tree cache for a service allowed substantially less memory. The repository already exposes cgroup memory readings in polylogue/core/metrics.py; the adaptive limit should use the smaller applicable host/cgroup limit, otherwise the new default still permits an OOM in constrained deployments.
Useful? React with 👍 / 👎.
| for attachment in session.attachments: | ||
| object_count += 1 | ||
| total_chars += _text_len(attachment.name) | ||
| total_chars += _text_len(attachment.path) |
There was a problem hiding this comment.
For browser-capture, Gemini/Drive, and extracted-content inputs, parsers populate ParsedAttachment.inline_bytes, which can contain the attachment's full decoded contents, but this attachment accounting adds only the fixed object overhead and a few text fields. A raw containing large inline attachments can therefore be admitted with an estimate far below its actual resident tree size and bypass the intended whole-cache bound; add len(attachment.inline_bytes) when present.
Useful? React with 👍 / 👎.
1 participant
Summary
Bounds
DaemonParseStage's prefetch cache by an estimated parsed-treebyte budget, in addition to the existing raw-payload byte budget, so a
whale-dense census page can no longer pin gigabytes of resident memory
in the cache regardless of the payload-bytes clamp.
Problem
DaemonParseStage's inflight admission budget andRawParsePrefetchCache's own admission gate both account raw PAYLOADbytes, because payload size is the only thing knowable before a raw is
parsed. But a parsed
ParsedSessiontree resident in the cache is notthe same size as the payload it came from -- Pydantic model instances,
per-block dicts, and general Python object overhead inflate a compact
JSON/JSONL payload substantially. Two earlyoom kills (19.3G and 20.2G
RSS peaks, 2026-07-20) happened on a whale-dense page precisely because
the cache retained a whole 2000-raw page of parsed trees while only raw
payload bytes were budgeted; clamping the inflight (pre-parse) budget
did nothing, since the pressure came from trees already sitting in the
cache post-parse, not from parses in flight.
Solution
polylogue/daemon/parse_prefetch.py:estimate_parsed_tree_bytes()-- a cheap, single linear-passstructural estimator over
list[ParsedSession]: sums text/contentfield lengths across sessions/messages/blocks/attachments/session
events and counts model-instance nodes, then applies two constants
(
_ESTIMATOR_BYTES_PER_CHAR=2,_ESTIMATOR_OBJECT_OVERHEAD_BYTES=1024)calibrated against a manual deep-object-graph measurement on
synthetic sessions (calibration data + fit in the comment above the
constants; test reproduces the measurement independently). Deliberately
NOT a recursive
sys.getsizeof/pympler-style deep walk -- that runs onwarm()'s hot path for every raw in a page (up to ~2000), so it hasto stay O(payload size) with low constant factor, not O(object graph).
daemon_parse_stage_max_cached_tree_bytes()-- a second adaptivebudget (1/8 of physical RAM, clamped
[256 MiB, 4 GiB], overridePOLYLOGUE_DAEMON_PARSE_STAGE_MAX_CACHED_TREE_BYTES), mirroring theexisting inflight-bytes budget's shape but sized for the bigger
(resident-tree) number.
DaemonParseStagetracks a side ledger (_tree_bytes_by_raw_id,_cached_tree_bytes_total) keyed by the same raw_ids asself.cache.RawParsePrefetchCacheitself is not touched orsubclassed -- it's a shared type
bulk_rebuild.pyhands directly toRebuildIndexRequest.prefetch_cache, so this stays additive ratherthan replacing it.
all (skips
cache.try_admitentirely, so it doesn't even consume apayload-bytes admission slot) -- the writer-held pass reparses it
normally, identical to any other prefetch miss.
insertion, via dict iteration order) through the raw cache's own
pop()once the running estimated total exceeds budget. Sinceexternal consumers (the writer-held pass) can also
pop()entriesdirectly, the ledger reconciles lazily against
self.cache.contains()before every eviction decision.
Verification
devtools test tests/unit/daemon/test_parse_prefetch.py-> 12 passed(7 pre-existing + 5 new: estimator monotonicity, estimator within 3x
of an independent deep-size measurement, whale-never-retained,
eviction-under-pressure, and the new budget's adaptive-clamp/env-override).
.venv/bin/python -m mypy --strict polylogue/daemon/parse_prefetch.py tests/unit/daemon/test_parse_prefetch.py->Success: no issues found in 1 source file(both files)devtools verify --quick-> all 16 stepsok, exit 0 (includesrender all --check/ topology / docs-coverage, since newmodule-level functions were added to an existing file -- no new
module, so no topology-projection regen was needed)
Ref polylogue-xb4i