Repository navigation
perf: engine reads plans and task indexes once per change - #1936
Merged
Merged
Conversation
Every Store read (Show, Task, Notes, ...) reloaded the whole plan from SQLite, and each plandb.Open read it twice more. Listing one conversation's tasks with 800 run rows therefore read the plan 800 times over. refresh now records SQLite's data_version and the size, mtime and inode of the database and its log before it reads, and skips the read while all of them stand still. A write by this handle or another still moves the stamp, so the next read sees it. Open takes the stamp before its own load. Measured with a synthetic 200-task plan: 200 Show calls allocated 153 MB before and under 3 MB after (TestRepeatedReadsOfAnUnchangedPlanLoadItOnce). Co-Authored-By: Claude Opus 5.5 <[email protected]>
Three costs that grew with the size of one conversation's history: - AppTasks opened the plan store once per run row, and every open read the whole plan. It now opens the read handles once for the listing. 150 run rows over a 150-task plan: 235 MB allocated before, under 16 MB after (TestAppTasksOpensThePlanOnceForAllTheRunRows). - markPlanInterrupted copied every run row for every plan row. It now walks the rows where they are kept; the listing is no longer quadratic. - Every conversation in a project decoded the project's whole task index. The decoded rows of the last four indexes (up to 4 MiB of file each) are kept under the file's stamp and shared; lastPerNode builds the slice each caller then orders. 40 readings of a 1,500-row index: 180 MB before, under 64 MB after (TestAnUnchangedTaskIndexIsDecodedOnce). The journal record cache also forgets what nobody reads: a kept record not touched for 30 s is dropped (RecordCache.Expire, armed by a timer while anything is kept), and it keeps at most 8 records instead of 32. Paging a chat still hits within seconds; a hundred journals viewed in a morning are no longer held until the app quits. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Port only the engine cache corrections from 531e2f1dc and c404aea7e: clear evicted index references, share the mtime/ctime settling guard, and target the earliest unleased journal deadline. Keep their deterministic regressions and log allocation fixture measurements. The project index already uses replacement-sensitive record stamps. No changes from 799d0baa2 are needed here. Document this limited subset and leave idle member release, app task-list disk caching, persistence write skipping and GC policy for follow-up work. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Returned task-index structs still shared nested files, crew and verdict storage with cached rows. Deep-copy those fields on return, including route prices, crew maps and nested slices. Concurrent misses decoded the same index independently. Coordinate in-flight reads by path and full file stamp, without admitting unsettled or oversized files. Changed stamps start separate reads and cannot be replaced by an old completion. AppTasks reopened the live plan for its plan projection and reread matching trajectories. Share handles and memoized trajectory projections within one listing. Keep only plan counts and requested app facts; release raw payloads before the next task and start fresh on the next listing. Deterministic red/green gates observed 16 -> 1 decodings and 2 -> 1 plan opens and matching trajectory reads. Added nested ownership, stamp independence, listing freshness and weak-reference payload-retention coverage. Existing allocation caps remain unchanged. Validation: 35 focused internal/session tests passed with -race -count=3 via make test-focus; six final regression tests also passed with those flags. An overlay restoring duplicate trajectory reads fails the read-count gate. git diff --check passes. The changed changelog entry validates individually; repository-wide changelog checking has the same 231 errors at the base. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Remove per-path singleflight coordination and its injected parser and tests. Concurrent misses decode independently again; retained-cache admission still requires a settled, unchanged file stamp. This removes decoder-panic waiters and stale sharing across unsettled same-stamp rewrites. Keep deep copies and cap caller-visible row slices at their lengths. A new regression reslices results to capacity, mutates reachable files, crew and verdict evidence, and verifies ordinary reads remain unchanged after limiting, deduplication and tombstone filtering. Preserve shared listing reads and their read-count assertion. Update the existing change entry and performance gates to describe independent misses. Validation: focused internal/session tests passed with -race -count=3 via make test-focus, including the new regression and listing read-count gate. The regression failed before the fix for excess capacity and cache mutation. Existing allocation budgets pass; make fmt-check and git diff --check pass. The changed changelog entry validates; full changelog checking has the same 231 errors at the base commit. Standards review: 0 findings. Spec review: 0 findings. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Deep-copy and seal every mutable Task slice so caller mutations cannot corrupt unchanged plan reads. Add recursive reflection coverage for future mutable fields and sequential and concurrent read isolation tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
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
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.
What changes
Low-risk engine work reduction on the desktop task views. No wire or behaviour change.
Measured
Engine process on an Apple silicon Mac, a copy of a large real home (211 agents, 2,638 tasks), same driver replaying the all-crews task view; fresh home copy per run.
Output is identical in all runs (211 agents, 2,638 tasks, 2,094 processes). Memory falls about 5%; CPU about 18%; a repeat pass of the task view takes half the time.
Not in this PR
Most of the remaining memory is crew members held open while their tasks are read, plus GC settings. An earlier attempt (#1933) brought the engine to about 250 MB but its idle-release and on-disk cache did not pass review; that work needs the release coordinated inside the session's own admission and stays in #1933 as a draft.
Validation
-race -count=3oninternal/plandb,internal/session,internal/filememo.go build ./...,go vet(alsoGOOS=windows) on touched packages.app/v3: no new test failures ininternal/session,internal/plandb,internal/filememoorcmd/codeaf. Twocmd/codeaftests that fail intermittently show the same rate on both (20 runs each).Known pre-existing issues (also on app/v3)
internal/sessiontest failures,TestCrewsOrganizeDiscoversInitiallyUnheldHostBeforeLocalNaming, and the changelog gate's existing errors.cmd/codeaftests end the whole test binary early ("still leaving after 30s"), so most of that package does not run locally.🤖 Generated with Claude Code