perf(session): reuse parsed journal during resume
Why: SessionManager.open() parses the complete journal for its header and then setSessionFile() parses the same journal again. Changes: - Reuse the entries already loaded by open() through a private setup path. - Keep the public setSessionFile() contract unchanged. Evidence: - A 19.98 MB, 12,000-entry journal improved from 48.011 ms to 25.406 ms median across 15 runs with identical restored state. Refs #8117
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed session resume fully reading and parsing the journal twice by reusing the entries already loaded by `SessionManager.open()` ([#8117](https://github.com/can1357/oh-my-pi/issues/8117)).
|
||||
|
||||
## [17.2.12] - 2026-08-08
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -1285,6 +1285,10 @@ export class SessionManager {
|
||||
|
||||
/** Switch to a different session file (resume / branch). */
|
||||
async setSessionFile(sessionFile: string): Promise<void> {
|
||||
await this.#setSessionFile(sessionFile);
|
||||
}
|
||||
|
||||
async #setSessionFile(sessionFile: string, loadedEntries?: FileEntry[]): Promise<void> {
|
||||
await this.#drainAndCloseWriter();
|
||||
this.#clearDiskError();
|
||||
this.#draftOnlySessionCleanupArmed = false;
|
||||
@@ -1294,7 +1298,7 @@ export class SessionManager {
|
||||
this.#rememberBreadcrumb(this.#cwd, resolvedSessionFile);
|
||||
|
||||
const titleSlot = await readTitleSlotFromFile(resolvedSessionFile, this.#storage);
|
||||
const fileEntries = await loadEntriesFromFile(resolvedSessionFile, this.#storage);
|
||||
const fileEntries = loadedEntries ?? (await loadEntriesFromFile(resolvedSessionFile, this.#storage));
|
||||
if (fileEntries.length === 0) {
|
||||
// Explicit but empty/missing path (e.g. --session flag): start fresh but
|
||||
// keep the requested path and materialize the header immediately.
|
||||
@@ -2601,7 +2605,7 @@ export class SessionManager {
|
||||
: path.dirname(path.resolve(filePath)));
|
||||
const manager = new SessionManager(cwd, dir, true, storage);
|
||||
manager.#suppressBreadcrumb = options?.suppressBreadcrumb === true;
|
||||
await manager.setSessionFile(filePath);
|
||||
await manager.#setSessionFile(filePath, loaded);
|
||||
return manager;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { FileSessionStorage } from "@oh-my-pi/pi-coding-agent/session/session-storage";
|
||||
import { removeWithRetries, TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
const tempDirs: TempDir[] = [];
|
||||
@@ -115,18 +116,28 @@ describe("SessionManager cwd adoption on resume", () => {
|
||||
expect(manager.getSessionDir()).toBe(path.resolve(launchSessions));
|
||||
});
|
||||
|
||||
it("falls back to the launch cwd when opening a session whose project directory is gone", async () => {
|
||||
it("falls back to the launch cwd with one full read when the recorded project directory is gone", async () => {
|
||||
const launch = makeTempDir("@pi-cwd-launch-");
|
||||
const store = makeTempDir("@pi-cwd-store-");
|
||||
const goneProject = makeTempDir("@pi-cwd-gone-");
|
||||
const file = await writeSession(goneProject, store);
|
||||
await removeWithRetries(goneProject);
|
||||
class CountingFileSessionStorage extends FileSessionStorage {
|
||||
fullReads = 0;
|
||||
|
||||
const manager = await SessionManager.open(file, undefined, undefined, { initialCwd: launch });
|
||||
override readText(filePath: string): Promise<string> {
|
||||
this.fullReads++;
|
||||
return super.readText(filePath);
|
||||
}
|
||||
}
|
||||
const storage = new CountingFileSessionStorage();
|
||||
|
||||
const manager = await SessionManager.open(file, undefined, storage, { initialCwd: launch });
|
||||
|
||||
expect(manager.getCwd()).toBe(path.resolve(launch));
|
||||
// /new and /branch anchor to the launch cwd, not the deleted project's store.
|
||||
expect(manager.getSessionDir()).toBe(SessionManager.getDefaultSessionDir(launch));
|
||||
expect(manager.getSessionDir()).not.toBe(path.resolve(store));
|
||||
expect(storage.fullReads).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user