perf(session): gate blob-ref resolution behind synchronous precheck
resolveBlobRefsInEntries handed every non-session entry to the recursive async resolvePersistedBlobRefs walk, allocating and awaiting child promises even for plain-text entries with no blob:sha256: refs. On large text-heavy histories this dominated the blob_resolve phase of session open. Add a cheap synchronous containsBlobRef precheck that early-exits on the first ref and allocates nothing. Interleave the precheck with per-entry initiation so positive entries still start resolution at the same relative point as the old filter+map schedule (a later entry that gains a ref during an earlier BlobStore.get is still scanned after that mutation). Blob-free N=5000 fixture: blob_resolve median 19.5ms -> 1.1ms, zero BlobStore.get calls. Fixes #5922
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Session load now skips the recursive async blob-ref resolver for entries with no `blob:sha256:` references. A cheap synchronous precheck gates the walk per entry (preserving the previous per-entry initiation order under synchronous store mutation), so text-heavy histories no longer pay the `Promise.all` tree descent for every non-session entry ([#5922](https://github.com/can1357/oh-my-pi/issues/5922)).
|
||||
|
||||
## [17.0.3] - 2026-07-17
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -271,10 +271,38 @@ async function resolvePersistedBlobRefs(value: unknown, blobStore: BlobStore, ke
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Cheap synchronous precheck: does this value's tree contain any `blob:sha256:` string?
|
||||
* Early-exits on the first hit and allocates no promises, so blob-free entries skip the
|
||||
* async {@link resolvePersistedBlobRefs} descent entirely. Conservative — a blob ref in a
|
||||
* non-resolved position still returns true, which only costs an extra (no-op) walk.
|
||||
*/
|
||||
function containsBlobRef(value: unknown): boolean {
|
||||
if (typeof value === "string") return isBlobRef(value);
|
||||
if (Array.isArray(value)) {
|
||||
for (const item of value) {
|
||||
if (containsBlobRef(item)) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
if (typeof value !== "object" || value === null) return false;
|
||||
for (const key in value) {
|
||||
if (containsBlobRef((value as Record<string, unknown>)[key])) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export async function resolveBlobRefsInEntries(entries: FileEntry[], blobStore: BlobStore): Promise<void> {
|
||||
await Promise.all(
|
||||
entries.filter(entry => entry.type !== "session").map(entry => resolvePersistedBlobRefs(entry, blobStore)),
|
||||
);
|
||||
const pending: Promise<void>[] = [];
|
||||
// Interleave precheck + initiation per entry so a positive entry begins resolution at the same
|
||||
// relative point as the old filter+map schedule (no scan-all-first pass that could observe a
|
||||
// later entry before an earlier resolution mutates it).
|
||||
for (const entry of entries) {
|
||||
if (entry.type === "session") continue;
|
||||
if (!containsBlobRef(entry)) continue;
|
||||
pending.push(resolvePersistedBlobRefs(entry, blobStore));
|
||||
}
|
||||
await Promise.all(pending);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -125,4 +125,46 @@ describe("session image persistence", () => {
|
||||
expect(resolvedImage?.data).toBe(data);
|
||||
expect(resolvedItem?.result).toBe(data);
|
||||
});
|
||||
|
||||
it("skips the async resolver for entries without blob refs while still resolving blob-ref entries", async () => {
|
||||
using tempDir = TempDir.createSync("@session-blob-precheck-");
|
||||
const blobStore = new BlobStore(tempDir.path());
|
||||
let getCalls = 0;
|
||||
const origGet = blobStore.get.bind(blobStore);
|
||||
blobStore.get = async (hash: string) => {
|
||||
getCalls++;
|
||||
return origGet(hash);
|
||||
};
|
||||
|
||||
const imageData = Buffer.alloc(1500, 7).toString("base64");
|
||||
const withImage = messageEntry({
|
||||
role: "toolResult",
|
||||
toolCallId: "call-1",
|
||||
toolName: "read",
|
||||
content: [png(imageData)],
|
||||
isError: false,
|
||||
timestamp: 0,
|
||||
} as unknown as ToolResultMessage);
|
||||
const persistedWithImage = prepareEntryForPersistence(withImage, blobStore);
|
||||
|
||||
const textOnly: FileEntry[] = Array.from({ length: 50 }, (_, i) => ({
|
||||
type: "message",
|
||||
id: `text-${i}`,
|
||||
parentId: i === 0 ? null : `text-${i - 1}`,
|
||||
timestamp: new Date(0).toISOString(),
|
||||
message: { role: "user", content: [text(`plain body ${i}`)], timestamp: 0 },
|
||||
})) as unknown as FileEntry[];
|
||||
|
||||
const loaded: FileEntry[] = [
|
||||
...textOnly.map(entry => structuredClone(entry)),
|
||||
structuredClone(persistedWithImage),
|
||||
];
|
||||
await resolveBlobRefsInEntries(loaded, blobStore);
|
||||
|
||||
// The blob-ref entry resolves through BlobStore.get exactly once; the 50 text entries never touch it.
|
||||
expect(getCalls).toBe(1);
|
||||
const resolved = loaded[loaded.length - 1] as ToolResultEntry;
|
||||
const resolvedImage = resolved.message.content.find((block): block is ImageContent => block.type === "image");
|
||||
expect(resolvedImage?.data).toBe(imageData);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user