Make watch ticks cheap - #321
Open
daaain wants to merge 5 commits into
Open
Conversation
`_enable_next_link_on_previous_page` guarded its work with
if "last-page" not in content: return False
which can never fire: every page inlines the rule that hides the link,
`.page-nav-link.next.last-page { display: none }`, so the bare substring
is always present. Each call therefore read the whole page back and ran
a DOTALL backtracking `subn` over it, on every page, on every
conversion.
For a one-shot convert that is amortised. Under `watch` it is the
dominant per-tick cost: on a 302-session/373MB archive with 22 combined
pages it re-read 253MB and burned ~5.2s of regex per tick, to change
nothing on 21 of the 22 pages.
`transcript.html` emits the nav block exactly once, in the page header
directly after the inlined stylesheet, so its offset is bounded by the
template's preamble rather than by the transcript -- across that archive
it never ended past 110KB. Reading a 512KB prefix and looking for the
closing delimiter answers the question for the whole file; a page whose
block falls outside the prefix takes the full read exactly as before.
The delimiters are also what tell the real nav block apart from the CSS
rule and from transcript content quoting the class name, which the bare
substring could not.
The nav fix-up over that archive goes 5.22s -> 0.23s.
Also give `serve --watch` the `--combined no` treatment the `watch`
command already documents: regenerating the combined pages is what
forces a tick to reload the project rather than the session that grew.
The combined pages from the startup conversion stay on disk and keep
serving; they stop tracking the live session until restart, which is the
trade `watch` already makes, and the startup banner now says so.
Together, one appended line on that archive: 7.6s -> 1.26s per tick.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQUsXyK2XYsimkB68jNcu4
Follow-up to the pagination nav fix-up commit, which gave `serve --watch` the `--combined no` treatment. The doc already scoped live updates to session pages, but a combined page open under `serve --watch` used to track the live session and now does not, so say so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`watch` owns a ParsedEntryStore for the life of its loop, so a tick can resume its parse from the bytes the previous tick already read. `serve --watch` converts the hierarchy instead, and `process_projects_hierarchy` had no way to accept one -- so every tick built a fresh store via `_make_entry_store` and re-parsed each changed file whole. The hierarchy fans stale projects out over a process pool, and a store holding parsed entries is no use across a `spawn`: shipping one would cost more than the re-parse it saves, and the worker's copy would die with the task. So the caller-owned store is handed to the *inline* conversion only, and stays out of `_conversion_kwargs` (whose contents the pool pickles unchanged). That is the watch steady state anyway -- one live project stale per tick means `resolved_jobs == 1` and no pool. Measured on a 23-file/6.8MB project, one appended line per tick: 0.21-0.23s before, 0.12-0.15s after. Session pages come out byte-identical to the same sequence run without the store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CONTRIBUTING documents three steps for regenerating snapshots -- purge stale bytecode, regenerate serially, re-run read-only to verify -- and records two occasions where doing only part of them left a damaged .ambr, once silently truncating ~6000 lines. The recipe ran only the middle step, so the documented hazard was still one forgotten flag away. Also correct docs/live-updates.md: session pages keep their combined back-link under `serve --watch`, since the page it points at was written by the startup conversion and is still served. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A session page's "View All Sessions" back-link was emitted according to this run's `--combined` setting. That made rendered content depend on the run's mode -- something the freshness check knows nothing about, as it compares only message_count and library_version. So the state stuck: a page rendered by a `--combined no` run (which is every watch tick) kept no link for good, and later full conversions saw a current file and skipped it. An archive ended up with some session pages linking and some not, decided only by which run happened to render each. Two halves. The link now follows whether the combined file *exists* rather than whether this run writes it: under `serve --watch` the startup conversion wrote one and it is still served, so the link stays valid and the mixed state stops arising at all. And migration 013 records per page what it was rendered with, so a real transition -- a project that had no combined page until a full run wrote one, or one whose combined page was deleted -- marks those pages stale and regenerates them. The check is free: `get_stale_sessions` already reads the whole html_cache table in one query and already stats every session file, so this is one more in-memory comparison (measured at 0.05% of the call) plus one stat per run. Deriving it by reading the rendered page would not be free -- the link sits past the inlined stylesheet, tens of KB in -- which is the mistake the pagination fix-up in this branch removes. Existing rows get NULL and are read as already-matching, so a populated cache does not mass-invalidate on upgrade (the 007/011 precedent). Pages a past `--combined no` run left link-less stay so until something else regenerates them, which is exactly today's behaviour. `is_transcript_stale` and `get_stale_sessions` are parallel implementations of the same checks, so the comparison itself lives in one shared helper rather than being written twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
Collaborator
|
First, a baseline on Linux: So it's not free to go through all the projects there either. Now on Windows: So, there's indeed a faster turnaround (93s instead of yesterday's 131.8s), but that's still too much. |
Owner
Author
|
Hmm, maybe your archive is just on another level altogether, but with this branch I'm getting: The cache db is 1.85 GB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Watch re-converts on every change, so a tick's cost is latency you feel. On a
302-session / 373MB archive, one appended line per tick: 7.6–8.3s → 1.26s.
1. Stop re-reading whole pages to fix pagination links.
_enable_next_link_on_previous_pageguarded onif "last-page" not in content,which can never fire — every page inlines the CSS rule containing
that string. So each call read the whole page back and ran a backtracking
regex over it, every page, every conversion, to change nothing on 21 of 22.
The nav block sits in the header just after the inlined stylesheet, so a
512KB prefix answers for the whole file; anything past it takes the full read
exactly as before. 5.22s → 0.23s.
2. Don't reload the project on
serve --watchticks. It now passes--combined no, aswatchalready defaults to — regenerating the combinedpages is what forces a tick to reload the whole project rather than the
session that grew. Those pages stay on disk and keep serving, but stop
tracking the live session until restart; the index still regenerates every
tick, so new sessions stay discoverable.
serve --watchalso now keeps itsparsed entries between ticks, as
watchalready did: 0.21s → 0.13s on a6.8MB project, with byte-identical output.
3. Fix: the combined back-link was sticky. Found while checking (2). The
link was emitted per the run's
--combinedsetting, so rendered contentdepended on something the freshness check doesn't track. A page rendered by a
--combined norun kept no link permanently — later full runs saw a currentfile and skipped it — leaving archives where some session pages link and some
don't, decided only by which run rendered each. The link now follows whether
the combined file exists, and migration 013 records each page's state so
real transitions regenerate. Costs one in-memory comparison (0.05% of the
staleness call) plus one stat per run; existing rows are NULL and read as
matching, so no mass invalidation on upgrade.
Also:
just update-snapshotnow runs the whole procedure CONTRIBUTINGdocuments — purge bytecode, regenerate serially, verify read-only. Partial
runs caused both
.ambrcorruption incidents recorded there.Testing.
just cigreen: 3194 unit, 68 TUI, 108 browser, 78 integration;ruff, pyright, ty. Regression tests for each fix, each verified to fail
against unpatched code.
Notes. Combined pages not tracking the live session under
serve --watchis deliberate, and documented in
docs/live-updates.md. A long-runningserve --watchnow holds parsed entries it used to discard, bounded by thestore's existing 268MB budget. Migration 013 is a plain in-place
ALTER TABLE,independent of the
library_versionbreaking-changes map — mergescleanly with
dev/agent-spawn-linking.Not in scope. Subagent sessions still rewrite every cache row per tick
(~63ms for 537 rows vs ~2.7ms to append 12). Not fixable in isolation: a
trunk's rows include its spliced subagent transcripts, so an appended file
doesn't mean appended rows, and rows read back in timestamp order with no
positional key. The fix is to stop splicing agent rows into trunk rows —
a separate change.