Repository navigation
Conversation
Tchips46
left a comment
There was a problem hiding this comment.
Review: OPFS cache (1652dd2)
Line numbers are at this PR's head. Content keyed by sha1 and checked on read, and trees keyed by user and project, make a wrong blob or a cross-project mix-up impossible. The risk is a stale path → hash entry in the client tree, which is now served straight from the cache.
Must fix
1. An older hash can overwrite a newer one, and is then served (packages/project/src/fs/project-fs.ts:96-97, :306, :243-249)
refresh()(after a rename, copy, restore or reconnect),_fetch'sreadManychunks and overlapping_applyRemotebatches all write the tree when their reply arrives. None checks whether the path changed while the request was out.- Example: a refresh lists
a.ts= H1. Meanwhile the user savesa.ts(H2, and the watcher reports H2). Then the listing's reply arrives and puts H1 back, and no later event fixes it. The next read finds H1's blob in OPFS and returns the text from before the save. - Before, a stale entry failed the mtime check and went to the server. Writers that read without
freshand send noexpectedHashthen overwrite the newer file: the git plugin's Ignore action (plugins/git/src/actions/create-actions.ts:110-113) appends to the old.gitignore. - Fix: a counter per path, bumped by
write,deleteand_applyRemote. Each reply skips any path whose counter moved since its request left. - Timing-dependent: the code path is confirmed, the race was not reproduced. A test with a write during an in-flight refresh would cover it.
Should fix
2. Opening a project now reads every file in full (packages/server-core/src/fs/fs-service.ts:158-166)
listhashes each listed file before it returns, andProjectFs.initializeawaits it. On the first open, or a fresh clone, the tree appears only after every file (up to 64 MiB each,nf_modulesand assets included) has been read. Before, open only stat'ed the files.- Fix: in
list, return only hashesFileHashesalready knows.readandreadManyalready record hashes, and files without one fall back to the size and mtime check.
3. stat fails, or hangs, when hashing fails (fs-service.ts:122-123, 150-153)
- A file that can be stat'ed but not read (EACCES, a Windows lock) makes
statthrow.rename,copy,restoreandmkdirreturnstat(to), so they throw after the disk operation already succeeded, and_applyRemotekeeps the old hash. - A named pipe counts as a file, and
createReadStreamblocks on it, solistnever resolves (plausible). - Fix:
info.isFile() ? await this._hashes.of(absolute, info).catch(() => undefined) : undefined, and only pushinfo.isFile()entries inlist.
4. FileHashes keeps one entry per version, evicts by insertion order, and saves on every read (packages/server-core/src/fs/file-hashes.ts:44-61)
- The key includes the version, so each save adds an entry and the old one stays. A hit doesn't move the entry up. The 50k cap is shared by every project.
- Past 50k, listings miss and read files again.
rememberalso schedules a full rewrite of the JSON (up to ~7 MB) on every RPC read, even when the hash was known. - Fix:
Map<absolutePath, { version, hash }>, and skip the save when the value didn't change.
5. A save waits for the cache write (project-fs.ts:183-184)
await this._contents.set(...)means an OPFS write, an IndexedDB transaction and, onQuotaExceededError, a full sweep behind a web lock another tab may hold. The server already wrote the file, butwrite()and the DocumentService save wait for all that.- Fix:
void this._contents.set(...).catch(() => undefined), as_fetchdoes.setstores the bytes in memory first, so an immediate read still hits.
Nice to have
- Quota sweeps (
packages/project/src/cache/project-caches.ts:248-259): up to 256 background sets each wait for their own full sweep onQuotaExceededError. Share the running sweep promise. - Orphan files (same lines): the OPFS file is written before its BLOBS record, so a tab closed in between leaves files no sweep ever sees. Write the record first.
_trees()(project-caches.ts:316-317) reads keys and values in two transactions. Another tab writing in between misaligns them. Use one transaction.- IndexedDB opens (
packages/project/src/cache/cache-db.ts):transactopens the database on every call, about 2k opens on a cold open of a 2k-file project. Cache theopenCacheDb()promise.
1652dd2 to
a344f00
Compare
25d747e to
18ae94d
Compare
- a reply that left before a save, a delete or a server change never puts back an older hash (per-path change clock) - a file the server changed is read from the server until its stat answers - stat and list hash regular files only, and a failed hash is no hash - file hashes are kept one per path, least recently used evicted first, saved only when they change; the trash is not hashed - a save does not wait for the content cache - one shared cache database connection, one sweep for a full quota, records written before files, trees read in one transaction
18ae94d to
d8bf688
Compare
What does this PR do?
Add cache file system with OPFS. Add a hash on files to know if the editor need to pull the file again.
How do you test this PR?