fix(core): skip writing deps cache if already up-to-date#34582
fix(core): skip writing deps cache if already up-to-date#34582FrozenPandaz merged 1 commit intomasterfrom
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit 300a826
☁️ Nx Cloud last updated this comment at |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| export function writeCacheIfStale( | ||
| cache: FileMapCache, | ||
| projectGraph: ProjectGraph, | ||
| sourceMaps: ConfigurationSourceMaps, | ||
| errors: ProjectGraphErrorTypes[] | ||
| ): void { | ||
| if (lastWrittenCacheMtimeMs !== undefined) { | ||
| try { | ||
| const currentMtimeMs = statSync(nxProjectGraph).mtimeMs; | ||
| if (currentMtimeMs === lastWrittenCacheMtimeMs) { | ||
| return; | ||
| } | ||
| } catch { | ||
| // File doesn't exist or can't be stat'd — proceed with write | ||
| } | ||
| } | ||
| writeCache(cache, projectGraph, sourceMaps, errors); | ||
| } |
There was a problem hiding this comment.
Critical logic flaw: writeCacheIfStale will skip writing when the in-memory graph has changed but the file hasn't been modified externally.
Scenario:
- Process computes Graph A, writes it, sets
lastWrittenCacheMtimeMs = T1 - Graph changes due to file modifications, daemon recomputes to Graph B
writeCacheIfStaleis called with new Graph B data- Checks file mtime (still T1 since this process last wrote it)
currentMtimeMs === lastWrittenCacheMtimeMs→ returns without writing- Graph B is never written to disk, cache is stale
The function assumes unchanged file = unchanged data, but the in-memory graph may have been recomputed with new data that needs persisting.
Fix: Compare a hash/signature of the graph data itself, not just the file timestamp:
export function writeCacheIfStale(...) {
// Need to track what data was last written and compare
// against current data, not just check file mtime
const currentDataHash = hashGraphData(cache, projectGraph, sourceMaps, errors);
if (lastWrittenDataHash === currentDataHash) {
return; // Same data, skip write
}
writeCache(cache, projectGraph, sourceMaps, errors);
lastWrittenDataHash = currentDataHash;
}| export function writeCacheIfStale( | |
| cache: FileMapCache, | |
| projectGraph: ProjectGraph, | |
| sourceMaps: ConfigurationSourceMaps, | |
| errors: ProjectGraphErrorTypes[] | |
| ): void { | |
| if (lastWrittenCacheMtimeMs !== undefined) { | |
| try { | |
| const currentMtimeMs = statSync(nxProjectGraph).mtimeMs; | |
| if (currentMtimeMs === lastWrittenCacheMtimeMs) { | |
| return; | |
| } | |
| } catch { | |
| // File doesn't exist or can't be stat'd — proceed with write | |
| } | |
| } | |
| writeCache(cache, projectGraph, sourceMaps, errors); | |
| } | |
| export function writeCacheIfStale( | |
| cache: FileMapCache, | |
| projectGraph: ProjectGraph, | |
| sourceMaps: ConfigurationSourceMaps, | |
| errors: ProjectGraphErrorTypes[] | |
| ): void { | |
| const currentDataHash = hashGraphData(cache, projectGraph, sourceMaps, errors); | |
| if (lastWrittenDataHash === currentDataHash) { | |
| return; // Same data, skip write | |
| } | |
| writeCache(cache, projectGraph, sourceMaps, errors); | |
| lastWrittenDataHash = currentDataHash; | |
| } | |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
8af63d5 to
274f22e
Compare
274f22e to
300a826
Compare
## Current Behavior Frequent write cache calls during tasks hashing results in delays ## Expected Behavior Cache is validated but only written when changed ## AI Summary This pull request introduces an optimization to the project graph cache writing logic, reducing unnecessary disk writes when serving repeated requests with unchanged graphs. The main change is the addition of a mechanism to track the cache file's modification time and only write to disk if the file has been externally modified or not written yet by the current process. Optimizations to cache writing: * Added `writeCacheIfStale` function in `nx-deps-cache.ts` to prevent redundant cache writes by checking the cache file's modification time before writing. This function is now used in the daemon's graph recomputation logic, replacing the previous unconditional write. [[1]](diffhunk://#diff-82bd1a5a7b7320ffc3233470f191782c054bf69a696dc16001d2c4b1d0b04963R285-R312) [[2]](diffhunk://#diff-d5bf3c66e62cac1884a071bf07fd1991320a3e62b07bfc03af3b9557b714c892L17-R17) [[3]](diffhunk://#diff-d5bf3c66e62cac1884a071bf07fd1991320a3e62b07bfc03af3b9557b714c892L136-R149) * Introduced `lastWrittenCacheMtimeMs` variable to track the last successful write's modification time, updated after each cache write. [[1]](diffhunk://#diff-82bd1a5a7b7320ffc3233470f191782c054bf69a696dc16001d2c4b1d0b04963R202-R208) [[2]](diffhunk://#diff-82bd1a5a7b7320ffc3233470f191782c054bf69a696dc16001d2c4b1d0b04963R256-R261) Codebase updates: * Updated imports in `nx-deps-cache.ts` to include `statSync` for file modification time checks. ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes # (cherry picked from commit e4f2d39)
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Frequent write cache calls during tasks hashing results in delays
Expected Behavior
Cache is validated but only written when changed
AI Summary
This pull request introduces an optimization to the project graph cache writing logic, reducing unnecessary disk writes when serving repeated requests with unchanged graphs. The main change is the addition of a mechanism to track the cache file's modification time and only write to disk if the file has been externally modified or not written yet by the current process.
Optimizations to cache writing:
writeCacheIfStalefunction innx-deps-cache.tsto prevent redundant cache writes by checking the cache file's modification time before writing. This function is now used in the daemon's graph recomputation logic, replacing the previous unconditional write. [1] [2] [3]lastWrittenCacheMtimeMsvariable to track the last successful write's modification time, updated after each cache write. [1] [2]Codebase updates:
nx-deps-cache.tsto includestatSyncfor file modification time checks.Related Issue(s)
Fixes #