Skip to content

Commit b2005d0

Browse files
committed
Stop rewriting the whole file library on every sidebar listing
Every listing bumped `thumbnailStoredAt` on every record it returned, and a bump rewrites the WHOLE record - bytes included - so a 60-file library rewrote 60 full records per refresh, several times per page load. Debounced to once a day. On a 30-day TTL that is indistinguishable from bumping on every read, and expiry is unchanged: a stale thumbnail is still cleared the moment it's seen. Rebased onto #7366 rather than main: that PR rewrote these exact call sites to keep maintenance writes away from blob-bodied records (which can wedge the object store on WebKit), so the two changes now compose - skip the risky records, and debounce the rest.
1 parent 62fe11a commit b2005d0

2 files changed

Lines changed: 147 additions & 4 deletions

File tree

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
2+
import "fake-indexeddb/auto";
3+
4+
import type { FileId } from "@app/types/file";
5+
6+
/**
7+
* A TTL bump is `put(record)`, and `record.data` is the file itself - there is
8+
* no partial update in IndexedDB. So "note that this thumbnail was used" used
9+
* to rewrite every byte of every file in the library, on every listing, and the
10+
* sidebar lists on mount and on every workbench change.
11+
*
12+
* These tests pin the debounce by counting writes, because nothing else fails
13+
* if it is removed: the behaviour is identical, just far more expensive. Each
14+
* asserts on a NON-EMPTY set of writes - "nothing was written" would pass
15+
* before the fire-and-forget bump had a chance to run.
16+
*/
17+
18+
const nativePut = IDBObjectStore.prototype.put;
19+
20+
/** Ids written back during a listing, in order. */
21+
let writes: FileId[] = [];
22+
23+
function countWrites() {
24+
IDBObjectStore.prototype.put = function (
25+
this: IDBObjectStore,
26+
value: unknown,
27+
key?: IDBValidKey,
28+
) {
29+
writes.push((value as { id: FileId }).id);
30+
return key === undefined
31+
? nativePut.call(this, value)
32+
: nativePut.call(this, value, key);
33+
} as typeof IDBObjectStore.prototype.put;
34+
}
35+
36+
const HOUR = 60 * 60 * 1000;
37+
const DAY = 24 * HOUR;
38+
39+
/**
40+
* A fresh service over an empty store. fake-indexeddb keeps its data for the
41+
* whole file, so without the clear each test would see the previous test's
42+
* records and its write count would depend on execution order.
43+
*/
44+
async function freshFileStorage() {
45+
vi.resetModules();
46+
const [{ fileStorage }, { createStirlingFile, createNewStirlingFileStub }] =
47+
await Promise.all([
48+
import("@app/services/fileStorage"),
49+
import("@app/types/fileContext"),
50+
]);
51+
await fileStorage.clearAll();
52+
53+
/** Store one file carrying a thumbnail recorded `ageMs` ago. */
54+
const store = async (name: string, ageMs: number) => {
55+
const file = new File(["%PDF-1.7 stirling"], name, {
56+
type: "application/pdf",
57+
});
58+
const stub = createNewStirlingFileStub(file);
59+
await fileStorage.storeStirlingFile(
60+
createStirlingFile(file, stub.id),
61+
stub,
62+
);
63+
// storeStirlingFile stamps `Date.now()`; age it directly so the test does
64+
// not depend on how the thumbnail got there.
65+
await fileStorage.updateFileMetadata(stub.id, {
66+
thumbnail: "data:image/webp;base64,AAAA",
67+
thumbnailStoredAt: Date.now() - ageMs,
68+
});
69+
return stub.id;
70+
};
71+
72+
return { fileStorage, store };
73+
}
74+
75+
beforeEach(() => {
76+
writes = [];
77+
});
78+
79+
afterEach(() => {
80+
IDBObjectStore.prototype.put = nativePut;
81+
});
82+
83+
describe("thumbnail TTL bump — the whole record is rewritten, so debounce it", () => {
84+
test("rewrites the record recorded a day ago and leaves the recent one alone", async () => {
85+
const { fileStorage, store } = await freshFileStorage();
86+
const recent = await store("recent.pdf", 1 * HOUR);
87+
const stale = await store("stale.pdf", 2 * DAY);
88+
89+
countWrites();
90+
await fileStorage.getLeafStirlingFileStubs();
91+
92+
// Exactly one write, and it is the stale one. Before the debounce both were
93+
// rewritten, which on real files is the whole library.
94+
await vi.waitFor(() => expect(writes).toEqual([stale]));
95+
expect(writes).not.toContain(recent);
96+
});
97+
98+
test("repeated listings rewrite at most once, not once per listing", async () => {
99+
const { fileStorage, store } = await freshFileStorage();
100+
const stale = await store("stale.pdf", 2 * DAY);
101+
102+
countWrites();
103+
await fileStorage.getLeafStirlingFileStubs();
104+
await vi.waitFor(() => expect(writes).toEqual([stale]));
105+
106+
// The first bump reset the stamp to now, so the next three are free. This
107+
// is the case that used to cost a full-library rewrite every time.
108+
for (let i = 0; i < 3; i++) await fileStorage.getLeafStirlingFileStubs();
109+
await new Promise((resolve) => setTimeout(resolve, 0));
110+
expect(writes).toEqual([stale]);
111+
});
112+
113+
test("an expired thumbnail is still cleared on the first listing that sees it", async () => {
114+
const { fileStorage, store } = await freshFileStorage();
115+
// Past the 30-day TTL: expiry must not be debounced away.
116+
const expired = await store("expired.pdf", 31 * DAY);
117+
118+
countWrites();
119+
const stubs = await fileStorage.getLeafStirlingFileStubs();
120+
await vi.waitFor(() => expect(writes).toEqual([expired]));
121+
122+
expect(stubs.find((s) => s.id === expired)?.thumbnailUrl).toBeUndefined();
123+
});
124+
125+
test("the same debounce applies to the all-files listing", async () => {
126+
const { fileStorage, store } = await freshFileStorage();
127+
await store("recent.pdf", 1 * HOUR);
128+
const stale = await store("stale.pdf", 2 * DAY);
129+
130+
countWrites();
131+
await fileStorage.getAllStirlingFileStubs();
132+
await vi.waitFor(() => expect(writes).toEqual([stale]));
133+
});
134+
});

frontend/editor/src/core/services/fileStorage.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { alert } from "@app/components/toast";
2222
* Contains all data needed for both StirlingFile and StirlingFileStub
2323
*/
2424
const THUMBNAIL_TTL_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
25+
/** Don't rewrite a record to slide its TTL more often than this. */
26+
const THUMBNAIL_TTL_REFRESH_MS = 24 * 60 * 60 * 1000; // 1 day
2527

2628
export interface StoredStirlingFileRecord extends BaseFileMetadata {
2729
// Blob since the large-file OOM fix (stored by reference, no JS-side copy);
@@ -220,6 +222,13 @@ class FileStorageService {
220222
return Date.now() - record.thumbnailStoredAt < THUMBNAIL_TTL_MS;
221223
}
222224

225+
/** Worth rewriting? A bump rewrites the whole record, bytes included, so on a
226+
* 30-day TTL once a day is indistinguishable from every read. */
227+
private thumbnailTTLIsStale(record: StoredStirlingFileRecord): boolean {
228+
if (!record.thumbnailStoredAt) return true;
229+
return Date.now() - record.thumbnailStoredAt > THUMBNAIL_TTL_REFRESH_MS;
230+
}
231+
223232
/** Fire-and-forget: bump thumbnailStoredAt (or clear expired thumbnail) for a set of ids. */
224233
private async bumpThumbnailTTL(ids: FileId[], clear = false): Promise<void> {
225234
const targets = ids.filter((id) => !this.unwritableRecords.has(id));
@@ -729,8 +738,8 @@ class FileStorageService {
729738
record.thumbnail &&
730739
maintenanceMayRewrite(record, this.blobValuesSupported)
731740
) {
732-
if (fresh) tobump.push(record.id);
733-
else toexpire.push(record.id);
741+
if (!fresh) toexpire.push(record.id);
742+
else if (this.thumbnailTTLIsStale(record)) tobump.push(record.id);
734743
}
735744
this.reportIfUnreadable(record);
736745
stubs.push({
@@ -828,8 +837,8 @@ class FileStorageService {
828837
record.thumbnail &&
829838
maintenanceMayRewrite(record, this.blobValuesSupported)
830839
) {
831-
if (fresh) tobump.push(record.id);
832-
else toexpire.push(record.id);
840+
if (!fresh) toexpire.push(record.id);
841+
else if (this.thumbnailTTLIsStale(record)) tobump.push(record.id);
833842
}
834843
this.reportIfUnreadable(record);
835844
leafStubs.push({

0 commit comments

Comments
 (0)