Repository navigation
perf(studio): timeline thumbnails come back from disk in the next session - #6676
Conversation
…sion Reopening a project re-read every video's index and re-decoded every strip: the frame cache and the source facts lived in memory, keyed by the session. Now each source's facts (aspect, duration) and every decoded frame are kept in Cache Storage under the file's ETag, the version the server says it is. A new session checks each source once with a conditional one-byte read: a 304 means the stored frames are still that file's, and a strip they cover paints without opening a reader. A new ETag drops what was stored for the old one. Sources are dropped whole, least recently used first, past 256 MiB. A store that is missing or fails (private window, quota) warns once and the strip decodes as before. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Edit accuracy: accurate 271 (base branch 271), smooth 129 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit · head 34a9762
The disk path ignores contentVersion, so a project reopened in the same page after a video was replaced shows the old file's frames. Repro below (passes on main, fails here). CI is green on this head.
Scope
Read all 6 files end to end, plus the preview route's ETag/304 handling (settledFileTag, exact If-None-Match match) and the scheduler timeout path. Ran thumbnailDiskCache.test.ts and thumbnailVideoDecoder.test.ts locally: 51 pass, plus the repro.
— Genesis
|
|
||
| function used(name: string, addedBytes = 0): void { | ||
| const caches = loadIndex().caches; | ||
| caches[name] = { usedAt: Date.now(), bytes: (caches[name]?.bytes ?? 0) + addedBytes }; |
There was a problem hiding this comment.
nit: byte counts only ever go up, and each tab writes its whole snapshot of the index, so the 256 MiB cap drifts. A frame stored again under the same name (a disk frame that accepts rejected for another clip's window, which then gets decoded again) has its bytes counted twice. A second tab saving its older snapshot drops the first tab's caches and etags entries, so those caches fall outside the LRU, and when they're used again their bytes start counting from 0. Subtract the replaced entry's size on put, and merge with a fresh localStorage read before saving.
There was a problem hiding this comment.
Mostly fixed in f35c9bc: the double count and the lost caches entries are both handled. One part is still open, and it doesn't block: in mergedIndex, mine.etags wins for every key, including keys this tab only loaded at startup. So if another tab moves a source to a new ETag, this tab's next save puts the old one back. I checked with a throwaway test: the other tab writes "two", this tab saves for another source, and the index holds "one". The next session then validates the old ETag, gets a 206 and decodes that source from scratch. It loses a disk hit but never paints wrong frames. Fix: record the keys rememberVersion sets (the same way as dropped) and let only those win over the saved ones.
… index counts a frame once The stored version a session validated was kept per video for the whole page, so reopening a project after one of its videos was replaced at the same path painted the old file's frames until a full reload. It is now kept per video and project open (its contentVersion): each open re-validates once. The index now counts a frame stored again under its name once, and merges with what other tabs saved before writing, keeping caches this tab dropped out, so the 256 MiB budget holds across tabs. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Summary
Timeline video thumbnails now survive a reload. #6629 shared one reader per video and kept released frames in memory, but all of it was keyed by the session. So reopening a project re-read every video's index (at a 3.4 GB offset for a long camera file) and decoded every strip again.
Now each source's facts (aspect and duration) and every decoded frame are kept in Cache Storage, under the file's ETag, which is the version the server says it is.
contentVersion) with a conditional one-byte read (Range: bytes=0-0,If-None-Match: <stored ETag>). A 304 means the stored frames are still that file's frames. A strip they cover then paints without opening a reader, reading the file or decoding anything. Frames come back into the shared memory cache, so sibling clips share them as before.mediaFetchnotes it), so a first visit costs no extra request. The info is stored once, and each frame is stored as it is decoded (fire-and-forget).localStoragekeeps the last ETag per source and each cache's size and last use. A frame stored again under its name counts once, and a save merges with what other tabs saved since, without bringing back caches this tab dropped.crypto.subtleis missing (no secure context), the store fails (quota, private window), or the ETag isn't exposed (a cross-origin source whose CORS doesn't listETag), Studio warns once and stays memory-only. Opening a cache creates it, so reads first checkcaches.has, and a read never leaves an empty cache behind for a version that is gone.Desktop serves Studio from a saved port (
studioPort.mjsreuses it across launches), so the origin, and with it this store, persists across app restarts. The main window uses Electron's default persistent session.Before
Right after a reload on
main, 3 of the 40 clips have their strip; the rest are still grey while every source is opened and decoded again (synthetic footage: three 10-minute 1080p test videos, 40 clips).After
The same moment after a reload with this branch: all 40 strips are painted from disk, and no source was opened.
What I measured
main: 697, 589, 435, 540 and 731 ms (median 589 ms). The disk index stayed frozen during these runs, so the main code really ran.main: a local SSD with perf(studio): share video sources and keep frames for timeline thumbnails #6629's sharing already in place. The saving grows with what a decode costs: hosted media, slower laptops, more clips.thumbnailDiskCache.test.ts(8) covers round trips, the 304 rule, dropping an old version, least-used eviction, a broken store, a re-stored frame counted once, and a save that keeps another tab's entries but not this tab's drops.thumbnailVideoDecoder.test.tsadds a second session (aftervi.resetModules()) that paints a strip withgetPrimaryVideoTrackand the canvas sink never called. A second test reopens the same project in the same page after its video is replaced (ETag"v1"→"v2", a 206) and expects the reader to open again (Genesis's repro). Studio player tests: 2421 pass.paints a strip from a past session's frames on disk, without opening the file. Eight mutations are each caught: no disk read, no disk write, no version from the reader, a session-dependent disk key, the stored version memoised per source only, no subtraction of a replaced frame's bytes, no merge on save, and drops not tracked.tsc --noEmit, oxlint, oxfmt, the comment ratchet and the pre-commit hooks (fallow) pass.What I did NOT exercise
ETag; otherwise they stay memory-only.putfailures are caught and warn once, but I didn't force one in a real browser.🤖 Generated with Claude Code