Skip to content

[RSI] fix(coding-agent): speed up session append and fork hot paths - #2276

Merged
sethkarten merged 2 commits into
mainfrom
rsi/session-manager-hot-path
Sep 14, 2026
Merged

sethkarten merged 2 commits into
mainfrom
rsi/session-manager-hot-path

Conversation

@sethkarten

@sethkarten sethkarten commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

What

SessionManager's append and fork hot paths pay per-entry costs that scale with session size (cycle B perf report, candidate C3):

  • _persist ran an O(entries) fileEntries.some(...) scan on every appended entry to answer "has an assistant message been persisted yet" — a boolean that can only flip false→true once. Sessions before their first assistant response (goal state, queued messages, harness/custom entries) pay the full scan per append: ~100 µs per append at 20k in-memory entries, all synchronous on the worker's event loop.
  • forkFrom wrote the forked file with one appendFileSync (open+write+close) per source entry: a 20k-entry fork stalled the caller's thread for ~0.5 s of syscalls.

Every message, tool result, bash execution, and agent status append in every resident session (including all in-process subagents) goes through _persist, so this is a per-turn, per-session tax.

Change

packages/coding-agent/src/core/session-manager.ts only:

  • New flip-once hasAssistantEntry cache. _appendEntry sets it on the first assistant message (O(1)); every fileEntries reassignment (setSessionFile/open, newSession, materializeSessionFile, rollback pop, createBranchedSession — both paths) refreshes it via _refreshHasAssistantEntry() (cold, once per reassignment). _persist now consults the cached flag instead of rescanning.
  • forkFrom writes the whole fork through a single descriptor: one openSync + per-entry writeSync + one closeSync, constant memory (no full-file join). createUniqueSessionFileTarget guarantees the target does not exist, so "w" creates it exactly like the first append used to. Failure semantics unchanged: on error the partial file stays behind and the next fork gets a fresh unique target.
  • The per-append existsSync(this.sessionFile) stat is deliberately kept, with a comment: it is what recovers a live session whose file was deleted underneath it — without it, appendFileSync would recreate a headerless stub. That recovery is pinned by test/session-manager/session-state.test.ts ("rewrites the full session if the session file disappears after flushing"). The benchmark below shows steady-state appends are dominated by the disk write, not this stat, so the candidate report's suggestion of caching it was dropped: it would break that pinned recovery for no measurable gain.

Micro-benchmark

npx tsx test/session-manager-append-bench.ts (committed; same machine, main vs this PR):

scenario main this PR
pre-assistant append, 1k entries 9 µs/append 1 µs/append
pre-assistant append, 5k 28 µs/append 1 µs/append
pre-assistant append, 20k 104 µs/append 1 µs/append
forkFrom 1k entries 41 ms 15 ms
forkFrom 5k 131 ms 30 ms
forkFrom 20k 462 ms 85 ms
steady-state append p50 (post-assistant) 24–26 µs 24–26 µs

Guard-path appends are now flat O(1) instead of O(entries); fork writes drop from O(entries) open/close pairs to one. forkFrom rows include the constant git-context subprocess cost on both sides. The steady-state row is unchanged by design: it is dominated by the (pinned) existence stat and the disk append itself.

Validation

  • npm run check (biome + tsgo + installer + browser-smoke): clean.
  • npx vitest run test/session-manager/ test/session-manager-flush.test.ts test/session-manager-git-state.test.ts: 171 passed — includes the pinned flush-order, no-assistant-guard, rollback, and deleted-file-recovery tests.
  • npx vitest run test/agent-session-recursion.test.ts test/daemon-supervisor-lazy-subagents.test.ts test/rlm-ledger.test.ts test/atomic-persistence.test.ts test/session-info-modified-timestamp.test.ts: 190 passed (fork consumers + persistence).
  • New test/session-manager/append-hot-path.test.ts (7 tests): pre-assistant suppression with full-history rewrite on the first assistant message; session_state persists pre-assistant while messages stay suppressed; flag recomputed on reopen; guard re-arms when branching to an assistant-less path; flag survives a rolled-back custom append; fork issues zero appendFileSync calls (spy) with git_state dropped and parents re-linked; forked pre-assistant sessions stay suppressed.

No open PR touches core/session-manager.ts (checked against the cycle B dedupe list and an open-PR title search).

This PR was authored autonomously as part of a recursive self-improvement program (RSI). Source report: ~/rsi/reports-cycle-b/perf.md, candidate C3.


Note

Medium Risk
Changes core session persistence invariants (pre-assistant guard and fork I/O); regressions could corrupt or suppress JSONL, though behavior is heavily tested and steady-state append semantics are intentionally preserved.

Overview
Speeds up session JSONL appends and forks by removing work that scaled with session size on every turn.

Append path: _persist no longer scans all in-memory entries on each append to decide whether an assistant message exists. A flip-once hasAssistantEntry cache is set on the first assistant append and recomputed only when the entry list is replaced (open, new session, branch, rollback, materialize). Pre-assistant suppression and session_state / session_info exceptions behave the same; the per-append existsSync check stays for deleted-file recovery.

Fork path: forkFrom writes the new file with one openSync, repeated writeSync via writeAllSync (handles short writes), and one closeSync instead of appendFileSync per copied line. Git-state dropping and parent re-linking are unchanged.

Adds a manual benchmark script, changelog note, and tests for guard/cache behavior, single-descriptor fork, and partial writeSync handling.

Reviewed by Cursor Bugbot for commit 5246257. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Speed up SessionManager append and fork hot paths

  • Caches the assistant-message check as a boolean flag on SessionManager, replacing a full fileEntries scan on every _persist call with constant-time lookup; the cache is maintained across loads, branches, resets, and rollbacks.
  • Replaces the per-line appendFileSync loop in SessionManager.forkFrom with a single openSync descriptor and a new writeAllSync helper that retries writeSync on partial writes and throws on no progress; the descriptor is closed in a finally block.
  • Adds benchmarks in session-manager-append-bench.ts and unit tests in append-hot-path.test.ts covering pre-assistant suppression, reopened/branched/rolled-back cache state, single-descriptor fork, and short-write recovery.
  • Risk: writeAllSync throws when writeSync makes no progress; forks that previously silently produced partial files now fail instead. Rollback in _appendEntry recomputes _refreshHasAssistantEntry, so any future code path that mutates fileEntries outside the existing append/load/branch/reset flows must also refresh the cache.

Macroscope summarized 5246257.

@github-actions

github-actions Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Prime Agent performance — completed

PR 5246257e compared with main 878410b3.

Overall: 0 regressed · 0 improved · 17 no clear change.

Metric Main This PR Change
Cold startup 2,262.9 ms 2,077.8 ms ≈ -185.1 ms (-8.18%)
Warm startup 1,378.0 ms 1,263.2 ms ≈ -114.8 ms (-8.33%)
Installation 18.55 s 15.49 s ≈ -3.06 s (-16.51%)
Compressed release artifacts 70.08 MB 70.10 MB ≈ +0.02 MB (+0.03%)
Installed footprint 571.97 MB 571.97 MB ≈ +0.0041 MB (+0.00%)
Idle memory, summed RSS 1,201.44 MB 1,203.66 MB ≈ +2.22 MB (+0.19%)

Python runtime

Metric Main This PR Change
Python kernel startup 172.9 ms 155.7 ms ≈ -17.1 ms (-9.92%)
Python cell round trip 0.869 ms 0.704 ms ≈ -0.165 ms (-19.03%)
Empty bash command 15.6 ms 14.8 ms ≈ -0.8 ms (-4.87%)
Bash git status 24.6 ms 20.9 ms ≈ -3.7 ms (-14.91%)
Bash 32 KiB output 17.0 ms 14.3 ms ≈ -2.8 ms (-16.21%)
35 cells / 9 shell calls 238.3 ms 210.4 ms ≈ -27.9 ms (-11.69%)
Python interrupt to done 2.107 ms 1.842 ms ≈ -0.264 ms (-12.54%)
Python state snapshot 35.1 ms 29.2 ms ≈ -5.9 ms (-16.69%)
Python state restore 473.7 ms 435.1 ms ≈ -38.6 ms (-8.14%)
Python idle RSS 35.21 MB 35.35 MB ≈ +0.14 MB (+0.38%)
Python RSS after pandas workload 97.34 MB 97.46 MB ≈ +0.12 MB (+0.12%)

Sandbox cost: ~$0.0947 — no inference calls.
Run, logs, and downloadable raw results

Methodology and samples

Main resolved at 2026-09-13T06:23:57.563551+00:00. Harness 878410b3.
Linux x64, 4 vCPU, 8 GB RAM, 20 GB disk; region us.
Image: node:24-bookworm@sha256:be23f54a88d34e8824c741b19b91064094f92c1c97b194144bfc8b50d67258e2.
Stock tools, skills, daemon, and Python bootstrap enabled; fresh homes and a fixed Git fixture.
Onboarding is dismissed; the editor starts without a selected model or submitted prompt.
Medians shown. Arrows require a 20% timing/memory change plus absolute floors and IQR.
These practical noise floors are not a statistical significance test.
Cold means stopped Prime processes; OS filesystem caches are not flushed.
No model requests or credentials. Installation excludes build/setup time.
Installer tarballs use loopback; npm/Python downloads use the network with fresh caches.
Artifact size counts release tarballs; footprint after first use includes registry packages.
MB is decimal. Summed RSS can double-count shared pages; PSS is recorded when available.
Provisioning, setup, and build durations are recorded separately in the raw results.
Kernel probes use the installed JSONL runtime, outside the TUI/TypeScript host.
Per trial: 50 Python cells, 5 calls per shell case, and one 35-cell mix (9 git status calls).
Cell/shell values are batch means; other runtime timings are single operations.
State fixture: a 10,000-row × 8-column integer DataFrame and a 10,000-integer list.
Restore runs in a fresh kernel, including pandas imports; kernel startup is excluded.
Kernel RSS covers the isolated Python process; loaded RSS follows the pandas workload.
Costs estimate full sandbox lifetimes at configured rates, including setup and build.
Budget target: $1; not a billing cap. Performance changes are informational.
Failed or incomplete execution fails the workflow; saved artifacts remain available.
Each side stops a phase after 2 identical consecutive failures.
Skipped trials are not attempted samples. Warm startup requires a successful cold launch.

Metric Main successful/attempted PR successful/attempted Main spread PR spread
Cold startup 10/10 10/10 IQR 165.1 ms IQR 344.9 ms
Warm startup 10/10 10/10 IQR 175.2 ms IQR 121.8 ms
Installation 3/3 3/3 range 2.48 s range 2.07 s
Compressed release artifacts 1/1 1/1 — —
Installed footprint 1/1 1/1 — —
Idle memory, summed RSS 10/10 10/10 IQR 13.06 MB IQR 8.43 MB
Python kernel startup 10/10 10/10 IQR 32.0 ms IQR 33.2 ms
Python cell round trip 10/10 10/10 IQR 0.153 ms IQR 0.104 ms
Empty bash command 10/10 10/10 IQR 2.6 ms IQR 1.4 ms
Bash git status 10/10 10/10 IQR 5.1 ms IQR 2.5 ms
Bash 32 KiB output 10/10 10/10 IQR 3.3 ms IQR 1.4 ms
35 cells / 9 shell calls 10/10 10/10 IQR 19.0 ms IQR 21.7 ms
Python interrupt to done 10/10 10/10 IQR 0.650 ms IQR 0.802 ms
Python state snapshot 10/10 10/10 IQR 8.5 ms IQR 9.3 ms
Python state restore 10/10 10/10 IQR 27.1 ms IQR 24.3 ms
Python idle RSS 10/10 10/10 IQR 4.43 MB IQR 3.56 MB
Python RSS after pandas workload 10/10 10/10 IQR 4.44 MB IQR 4.32 MB

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6104585. Configure here.

Review fix (high): forkFrom issued one writeSync per JSONL line and
ignored the returned byte count, so a short write left a torn line that
the next reopen repaired as crash damage, silently dropping forked
entries. Route the header and entry writes through a writeAllSync helper
that loops until the whole buffer lands, matching the short-write
contract writeFileAtomicSync already enforces.

Regression test: inject a partial-write descriptor (at most 3 bytes land
per buffer writeSync call, 1 byte for the string form) and assert the fork
completes with the full header and both messages on disk and a fresh
reopen loads the entries with parents intact. Against the
ignore-the-count code the test fails with 1 recoverable line instead of
3.

Review fix (low): retarget the forkFrom comment, the fork test title, and
the bench header at the current single-descriptor write path instead of
the removed per-entry appendFileSync path; the bench bullets now describe
the flip-once guard cache and the one-descriptor fork.
@sethkarten
sethkarten enabled auto-merge (squash) September 13, 2026 20:39
@sethkarten
sethkarten merged commit 5c42f85 into main Sep 14, 2026
26 checks passed
@sethkarten
sethkarten deleted the rsi/session-manager-hot-path branch September 14, 2026 06:24
@sethkarten
sethkarten restored the rsi/session-manager-hot-path branch September 17, 2026 07:00
@kevinjosethomas
kevinjosethomas deleted the rsi/session-manager-hot-path branch September 30, 2026 01:44
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.

2 participants