mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
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.
This commit is contained in:
@@ -0,0 +1,134 @@
|
||||
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
|
||||
import "fake-indexeddb/auto";
|
||||
|
||||
import type { FileId } from "@app/types/file";
|
||||
|
||||
/**
|
||||
* A TTL bump is `put(record)`, and `record.data` is the file itself - there is
|
||||
* no partial update in IndexedDB. So "note that this thumbnail was used" used
|
||||
* to rewrite every byte of every file in the library, on every listing, and the
|
||||
* sidebar lists on mount and on every workbench change.
|
||||
*
|
||||
* These tests pin the debounce by counting writes, because nothing else fails
|
||||
* if it is removed: the behaviour is identical, just far more expensive. Each
|
||||
* asserts on a NON-EMPTY set of writes - "nothing was written" would pass
|
||||
* before the fire-and-forget bump had a chance to run.
|
||||
*/
|
||||
|
||||
const nativePut = IDBObjectStore.prototype.put;
|
||||
|
||||
/** Ids written back during a listing, in order. */
|
||||
let writes: FileId[] = [];
|
||||
|
||||
function countWrites() {
|
||||
IDBObjectStore.prototype.put = function (
|
||||
this: IDBObjectStore,
|
||||
value: unknown,
|
||||
key?: IDBValidKey,
|
||||
) {
|
||||
writes.push((value as { id: FileId }).id);
|
||||
return key === undefined
|
||||
? nativePut.call(this, value)
|
||||
: nativePut.call(this, value, key);
|
||||
} as typeof IDBObjectStore.prototype.put;
|
||||
}
|
||||
|
||||
const HOUR = 60 * 60 * 1000;
|
||||
const DAY = 24 * HOUR;
|
||||
|
||||
/**
|
||||
* A fresh service over an empty store. fake-indexeddb keeps its data for the
|
||||
* whole file, so without the clear each test would see the previous test's
|
||||
* records and its write count would depend on execution order.
|
||||
*/
|
||||
async function freshFileStorage() {
|
||||
vi.resetModules();
|
||||
const [{ fileStorage }, { createStirlingFile, createNewStirlingFileStub }] =
|
||||
await Promise.all([
|
||||
import("@app/services/fileStorage"),
|
||||
import("@app/types/fileContext"),
|
||||
]);
|
||||
await fileStorage.clearAll();
|
||||
|
||||
/** Store one file carrying a thumbnail recorded `ageMs` ago. */
|
||||
const store = async (name: string, ageMs: number) => {
|
||||
const file = new File(["%PDF-1.7 stirling"], name, {
|
||||
type: "application/pdf",
|
||||
});
|
||||
const stub = createNewStirlingFileStub(file);
|
||||
await fileStorage.storeStirlingFile(
|
||||
createStirlingFile(file, stub.id),
|
||||
stub,
|
||||
);
|
||||
// storeStirlingFile stamps `Date.now()`; age it directly so the test does
|
||||
// not depend on how the thumbnail got there.
|
||||
await fileStorage.updateFileMetadata(stub.id, {
|
||||
thumbnail: "data:image/webp;base64,AAAA",
|
||||
thumbnailStoredAt: Date.now() - ageMs,
|
||||
});
|
||||
return stub.id;
|
||||
};
|
||||
|
||||
return { fileStorage, store };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
writes = [];
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
IDBObjectStore.prototype.put = nativePut;
|
||||
});
|
||||
|
||||
describe("thumbnail TTL bump — the whole record is rewritten, so debounce it", () => {
|
||||
test("rewrites the record recorded a day ago and leaves the recent one alone", async () => {
|
||||
const { fileStorage, store } = await freshFileStorage();
|
||||
const recent = await store("recent.pdf", 1 * HOUR);
|
||||
const stale = await store("stale.pdf", 2 * DAY);
|
||||
|
||||
countWrites();
|
||||
await fileStorage.getLeafStirlingFileStubs();
|
||||
|
||||
// Exactly one write, and it is the stale one. Before the debounce both were
|
||||
// rewritten, which on real files is the whole library.
|
||||
await vi.waitFor(() => expect(writes).toEqual([stale]));
|
||||
expect(writes).not.toContain(recent);
|
||||
});
|
||||
|
||||
test("repeated listings rewrite at most once, not once per listing", async () => {
|
||||
const { fileStorage, store } = await freshFileStorage();
|
||||
const stale = await store("stale.pdf", 2 * DAY);
|
||||
|
||||
countWrites();
|
||||
await fileStorage.getLeafStirlingFileStubs();
|
||||
await vi.waitFor(() => expect(writes).toEqual([stale]));
|
||||
|
||||
// The first bump reset the stamp to now, so the next three are free. This
|
||||
// is the case that used to cost a full-library rewrite every time.
|
||||
for (let i = 0; i < 3; i++) await fileStorage.getLeafStirlingFileStubs();
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
expect(writes).toEqual([stale]);
|
||||
});
|
||||
|
||||
test("an expired thumbnail is still cleared on the first listing that sees it", async () => {
|
||||
const { fileStorage, store } = await freshFileStorage();
|
||||
// Past the 30-day TTL: expiry must not be debounced away.
|
||||
const expired = await store("expired.pdf", 31 * DAY);
|
||||
|
||||
countWrites();
|
||||
const stubs = await fileStorage.getLeafStirlingFileStubs();
|
||||
await vi.waitFor(() => expect(writes).toEqual([expired]));
|
||||
|
||||
expect(stubs.find((s) => s.id === expired)?.thumbnailUrl).toBeUndefined();
|
||||
});
|
||||
|
||||
test("the same debounce applies to the all-files listing", async () => {
|
||||
const { fileStorage, store } = await freshFileStorage();
|
||||
await store("recent.pdf", 1 * HOUR);
|
||||
const stale = await store("stale.pdf", 2 * DAY);
|
||||
|
||||
countWrites();
|
||||
await fileStorage.getAllStirlingFileStubs();
|
||||
await vi.waitFor(() => expect(writes).toEqual([stale]));
|
||||
});
|
||||
});
|
||||
@@ -22,6 +22,8 @@ import { alert } from "@app/components/toast";
|
||||
* Contains all data needed for both StirlingFile and StirlingFileStub
|
||||
*/
|
||||
const THUMBNAIL_TTL_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
|
||||
/** Don't rewrite a record to slide its TTL more often than this. */
|
||||
const THUMBNAIL_TTL_REFRESH_MS = 24 * 60 * 60 * 1000; // 1 day
|
||||
|
||||
export interface StoredStirlingFileRecord extends BaseFileMetadata {
|
||||
// Blob since the large-file OOM fix (stored by reference, no JS-side copy);
|
||||
@@ -220,6 +222,13 @@ class FileStorageService {
|
||||
return Date.now() - record.thumbnailStoredAt < THUMBNAIL_TTL_MS;
|
||||
}
|
||||
|
||||
/** Worth rewriting? A bump rewrites the whole record, bytes included, so on a
|
||||
* 30-day TTL once a day is indistinguishable from every read. */
|
||||
private thumbnailTTLIsStale(record: StoredStirlingFileRecord): boolean {
|
||||
if (!record.thumbnailStoredAt) return true;
|
||||
return Date.now() - record.thumbnailStoredAt > THUMBNAIL_TTL_REFRESH_MS;
|
||||
}
|
||||
|
||||
/** Fire-and-forget: bump thumbnailStoredAt (or clear expired thumbnail) for a set of ids. */
|
||||
private async bumpThumbnailTTL(ids: FileId[], clear = false): Promise<void> {
|
||||
const targets = ids.filter((id) => !this.unwritableRecords.has(id));
|
||||
@@ -729,8 +738,8 @@ class FileStorageService {
|
||||
record.thumbnail &&
|
||||
maintenanceMayRewrite(record, this.blobValuesSupported)
|
||||
) {
|
||||
if (fresh) tobump.push(record.id);
|
||||
else toexpire.push(record.id);
|
||||
if (!fresh) toexpire.push(record.id);
|
||||
else if (this.thumbnailTTLIsStale(record)) tobump.push(record.id);
|
||||
}
|
||||
this.reportIfUnreadable(record);
|
||||
stubs.push({
|
||||
@@ -828,8 +837,8 @@ class FileStorageService {
|
||||
record.thumbnail &&
|
||||
maintenanceMayRewrite(record, this.blobValuesSupported)
|
||||
) {
|
||||
if (fresh) tobump.push(record.id);
|
||||
else toexpire.push(record.id);
|
||||
if (!fresh) toexpire.push(record.id);
|
||||
else if (this.thumbnailTTLIsStale(record)) tobump.push(record.id);
|
||||
}
|
||||
this.reportIfUnreadable(record);
|
||||
leafStubs.push({
|
||||
|
||||
Reference in New Issue
Block a user