fix(app): reuse hydrated composer history blobs - #46761
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The caching/coalescing change is localized, preserves existing semantics on missing/failed reads, and is backed by focused automated tests plus a manual benchmark harness.
Pull request overview
This PR reduces redundant IndexedDB blob reads during composer history hydration by reusing already-live object URLs and deduplicating concurrent blob loads for the same blob id, improving efficiency for histories with many image references.
Changes:
- Update the draft-store decode path to consult the live URL cache before reading blobs and to coalesce concurrent
getBlob(id)calls. - Add browser-level tests covering concurrent hydration, repeated references, remount behavior, missing blobs, and failed reads.
- Add a manual Playwright/Vite benchmark fixture to measure blob read counts/bytes and capture hydration timing metrics for cold vs warm mounts.
File summaries
| File | Description |
|---|---|
| packages/app/src/runtime/persistence/drafts.ts | Reuses cached object URLs and deduplicates in-flight blob reads during decode to avoid redundant IndexedDB work. |
| packages/app/test-browser/draft-history-cache.test.ts | Adds targeted tests validating URL reuse and coalesced blob reads across concurrent and repeated hydration cases. |
| packages/app/e2e/performance/composer-history/vite.config.ts | Adds a Vite build config for the manual composer-history hydration benchmark fixture. |
| packages/app/e2e/performance/composer-history/playwright.config.ts | Adds a Playwright config to run the manual benchmark with a controlled preview server and output directory. |
| packages/app/e2e/performance/composer-history/index.html | Provides the minimal HTML entrypoint for the benchmark fixture. |
| packages/app/e2e/performance/composer-history/fixture.tsx | Implements the production-component benchmark fixture, seeds IndexedDB, and records blob/document read mechanism metrics. |
| packages/app/e2e/performance/composer-history/composer-history.bench.ts | Adds the benchmark test driver that mounts cold/warm, validates recall + image decode, and reports samples. |
| packages/app/e2e/performance/composer-history/README.md | Documents benchmark scope, workload, metrics, and how to run it from packages/app. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Problem
decodepath. For each image reference,decodecalleddriver.getBlob(id)and only then consulted the module-levelurlscache inblobUrl.Blobobjects, even when all 50 URLs were already live from a previous mount, and even when the 50 references pointed at only 5 distinct images.Change
packages/app/src/runtime/persistence/drafts.ts: consult the live URL map before reading, and coalesce concurrent reads of the same id.test-browser/draft-history-cache.test.ts(concurrent history/draft reads, repeated references in one document, latest document after remount, just-stored attachments, missing blobs, failed reads, distinct ids).e2e/performance/composer-historybenchmark (productionComposerEditor,createComposerEditor,createComposerHistory, persistence codec, browser IndexedDB draft store).Benchmark
unique= 50 distinct images (12.43 MB stored),repeated= 50 references to 5 images (1.22 MB stored).texthas no images and acts as the control.historyReadyMs); the test then verifies ArrowUp recall and a decoded image before recording.335e4cavs candidatec8c0b4c, frozen production bundles, counterbalanced ABBA order (baseline, candidate, candidate, baseline), 10 repeats per block, 20 samples per cell. Chromium 147, Windows.Mechanism (identical in all 20 samples per cell)
History availability,
historyReadyMsmedian (p95), baseline → candidaterecallObservedMs(ArrowUp → correct text and decoded image, includes Playwright overhead) also stayed within the control band.Bloballocation, not a measured latency change.Per-block
historyReadyMsmedians (10 samples each)Desktop path (Electron MessagePort + main-process SQLite)
Added after review to close the desktop evidence gap. Same workload and ABBA protocol, but every blob crosses the production desktop path: renderer
api.ts→ipc-client.ts(Effect RPC, MessagePack) → ElectronMessagePort→IpcServerProtocolLive→storageHandlers→DesktopStorage→node:sqlite.DesktopStorage.layer, storage/event RPC handlers,RpcServer+IpcServerProtocolLive, and the real preload; renderer runs the realapi.ts,ipc-client.ts,createDesktopStorage, and the same production composer editor/history fixture. Fixture-owneduserDataper launch; seeded throughDraftsPutBlob/DraftsSetand flushed to SQLite before the cold mount. No app shell, service, updater, or menus.unique, 50 images) / 1.22 MB (repeated, 5 images); documents 77 KB. Renderer bundles frozen per revision (baselinedrafts.tsc16694a7, candidate544f21dd); main bundle, preload, and Electron shared. 10 repeats per block, 20 samples per cell, Windows.Per mount, main → renderer (identical in all 20 samples per cell)
DraftsGetBlobRPCs12.49 MB of blobs cross as 16.66 MB because
RpcSerialization.layerMsgPackuses the JSON codec, soSchema.Uint8Arraytravels as base64 inside MessagePack (×4/3). Pre-existing; not changed here.Median (p95), baseline → candidate
process.cpuUsage()user + system over the mount window (Windows ~16 ms tick). SQLite read + copy is the wall time insidedrafts.getBlob(query plusnew Uint8Array(data).buffer). Peak working set is the natural (no forced GC)peakWorkingSetSizeincrease of the renderer process above its pre-mount peak fromapp.getAppMetrics(); it shows the transient RSS spike of the base64 → string →Uint8Array→slice→Blobchain, not retained memory.unique / colddoes identical work in both builds (50 reads, 16.74 MB). The candidate blocks ran ~25% slower, but main-only metrics for identical main work moved by the same ratio across blocks (SQLite read 28/33 → 39/39 ms; main CPU 875/1016 → 1156/1227 ms) and the text control moved the same direction, so this is machine drift during the middle ABBA blocks, not a build effect. Treat that row as unchanged.Scope and limitations
clonePromptshallow-copies image parts and async submit later callsblobDataUrlon them, so unmount and document deletion are not safe last-consumer boundaries; that needs a separate ownership contract.