mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 16:02:15 +00:00
fix(terminal): correct the fence justification and key the cache by window
The markWritten docblock claimed "zero bytes cannot mutate the buffer". That
is false, and I verified it: `_core.writeSync('')` drains xterm's pending queue
and applies it. The exemption is still correct, but for a different reason —
a fence cannot introduce an *unattributed* mutation, because any bytes it
drains belong to a queued async write whose own completion callback bumps
first. The two write regimes are exhaustive: with writeSync present nothing
can queue, without it every write is async and self-bumps. A comment asserting
a false invariant is worse than no comment, since the next change may rely on
it, so it now states the real one.
Key the cache by scrollback window instead of a single slot. Consumers ask for
different windows against the same emulator — attach passes the full window
while agent/text reads pass 0 — so one slot thrashed to a 0% hit rate whenever
they alternated, silently removing the benefit on runtime-side emulators. Two
entries cover every caller pair in the tree.
Drop the epoch counter: markMutated already nulls the retained entry and an
entry is only ever stored under the current epoch, so the comparison could
never fail. Invalidation is simply "clear the cache".
This commit is contained in:
@@ -34,7 +34,7 @@ describe('HeadlessEmulator snapshot cache', () => {
|
||||
expect(second.scrollbackAnsi).toBe(first.scrollbackAnsi)
|
||||
})
|
||||
|
||||
it('re-serializes for a different scrollbackRows window', async () => {
|
||||
it('re-serializes the first time a different scrollbackRows window is asked for', async () => {
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('hello world')
|
||||
emulator.getSnapshot({ scrollbackRows: 100 })
|
||||
@@ -45,6 +45,22 @@ describe('HeadlessEmulator snapshot cache', () => {
|
||||
expect(serialize.calls()).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('keeps two alternating scrollback windows warm', async () => {
|
||||
// Why: attach asks for the full window while agent/text reads ask for 0,
|
||||
// and a single-slot cache thrashes to a 0% hit rate when they alternate.
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('alternating windows')
|
||||
emulator.getSnapshot({ scrollbackRows: 0 })
|
||||
emulator.getSnapshot()
|
||||
const serialize = spyOnSerialize(emulator)
|
||||
|
||||
emulator.getSnapshot({ scrollbackRows: 0 })
|
||||
emulator.getSnapshot()
|
||||
emulator.getSnapshot({ scrollbackRows: 0 })
|
||||
|
||||
expect(serialize.calls()).toBe(0)
|
||||
})
|
||||
|
||||
it('reflects an async write that lands after a cached snapshot', async () => {
|
||||
emulator = new HeadlessEmulator({ cols: 80, rows: 24 })
|
||||
await emulator.write('first line')
|
||||
|
||||
@@ -159,12 +159,16 @@ export class HeadlessEmulator {
|
||||
}
|
||||
|
||||
/**
|
||||
* Bumps only for real bytes. Why: flushParsedWrites() is a zero-byte write
|
||||
* used purely as a parse fence (session-output-plane.ts), and every
|
||||
* getSettledSnapshot runs one. Zero bytes cannot mutate the buffer — the OSC
|
||||
* and mouse-mode scans are no-ops and the escape tail is idempotent — so
|
||||
* bumping would evict the cache on every checkpoint read. Any write a fence
|
||||
* orders behind has already bumped on its own completion.
|
||||
* Bumps only for real bytes. Why this is safe even though a zero-byte write
|
||||
* is NOT inert — `_core.writeSync('')` drains xterm's pending queue and
|
||||
* applies it (verified) — is that a fence can never introduce an
|
||||
* unattributed mutation. Any bytes it drains belong to a queued async write,
|
||||
* and xterm runs that write's completion callback first, which bumps. The
|
||||
* two write regimes are exhaustive: with writeSync present every write takes
|
||||
* the sync path and nothing can queue; without it every write is async and
|
||||
* self-bumps. Fences are exempt because flushParsedWrites() is one, and
|
||||
* every getSettledSnapshot runs it — bumping would evict the cache on each
|
||||
* checkpoint read.
|
||||
*/
|
||||
private markWritten(data: string): void {
|
||||
if (data.length > 0) {
|
||||
|
||||
@@ -38,6 +38,9 @@ type CachedParts = {
|
||||
// (MAX_COLD_RESTORE_CACHE_BYTES) so the two budgets read in one unit.
|
||||
const MAX_CACHED_SNAPSHOT_BYTES = 4 * 1024 * 1024
|
||||
|
||||
/** Distinct scrollback windows retained per emulator. */
|
||||
const MAX_CACHED_SNAPSHOT_WINDOWS = 2
|
||||
|
||||
// Why code units: bounds V8 string storage without rescanning or flattening
|
||||
// multi-MB ropes — same sizing rule as getColdRestorePayloadBytes.
|
||||
function retainedSnapshotBytes(parts: CachedParts): number {
|
||||
@@ -55,31 +58,35 @@ export type HeadlessSnapshotSource = {
|
||||
}
|
||||
|
||||
export class HeadlessSnapshotCache {
|
||||
private epoch = 0
|
||||
private retained: (CachedParts & { epoch: number; scrollbackRows: number | undefined }) | null =
|
||||
null
|
||||
// Why keyed and not a single slot: consumers ask for different scrollback
|
||||
// windows against the same emulator — attach passes the full window while
|
||||
// agent/text reads pass 0 — and one slot thrashes to a 0% hit rate when they
|
||||
// alternate. Two covers every caller pair in the tree; a third evicts the
|
||||
// oldest rather than growing per emulator.
|
||||
private readonly entries = new Map<number | undefined, CachedParts>()
|
||||
|
||||
/** Invalidates the cache. Called for every mutation of a memoized part;
|
||||
* fields build() re-reads per call (cwd, lastTitle, escape tail) do not. */
|
||||
markMutated(): void {
|
||||
this.epoch += 1
|
||||
this.retained = null
|
||||
this.entries.clear()
|
||||
}
|
||||
|
||||
/** Builds a caller-owned snapshot, reusing the memoized serialize on a hit. */
|
||||
build(source: HeadlessSnapshotSource, scrollbackRows: number | undefined): TerminalSnapshot {
|
||||
const retained = this.retained
|
||||
let parts: CachedParts
|
||||
if (retained && retained.epoch === this.epoch && retained.scrollbackRows === scrollbackRows) {
|
||||
parts = retained
|
||||
} else {
|
||||
let parts = this.entries.get(scrollbackRows)
|
||||
if (!parts) {
|
||||
parts = computeCachedParts(source, scrollbackRows)
|
||||
// Why size-gated: see MAX_CACHED_SNAPSHOT_BYTES. Declining to retain costs
|
||||
// the pre-existing serialize, never correctness.
|
||||
this.retained =
|
||||
retainedSnapshotBytes(parts) <= MAX_CACHED_SNAPSHOT_BYTES
|
||||
? { ...parts, epoch: this.epoch, scrollbackRows }
|
||||
: null
|
||||
if (retainedSnapshotBytes(parts) <= MAX_CACHED_SNAPSHOT_BYTES) {
|
||||
if (this.entries.size >= MAX_CACHED_SNAPSHOT_WINDOWS) {
|
||||
const oldest = this.entries.keys().next()
|
||||
if (!oldest.done) {
|
||||
this.entries.delete(oldest.value)
|
||||
}
|
||||
}
|
||||
this.entries.set(scrollbackRows, parts)
|
||||
}
|
||||
}
|
||||
// Why cloned: a hit hands back the retained entry, so a caller mutating
|
||||
// its snapshot would otherwise corrupt every later one.
|
||||
|
||||
Reference in New Issue
Block a user