Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
nxProjectGraph,
readFileMapCache,
writeCache,
writeCacheIfStale,
} from '../../project-graph/nx-deps-cache';
import {
retrieveProjectConfigurations,
Expand Down Expand Up @@ -98,6 +99,7 @@ export async function getCachedSerializedProjectGraphPromise(): Promise<Serializ
waitPeriod = 100;
await resetInternalStateIfNxDepsMissing();
const plugins = await getPlugins();
const previousPromise = cachedSerializedProjectGraphPromise;
if (collectedUpdatedFiles.size == 0 && collectedDeletedFiles.size == 0) {
if (!cachedSerializedProjectGraphPromise) {
cachedSerializedProjectGraphPromise =
Expand All @@ -117,6 +119,8 @@ export async function getCachedSerializedProjectGraphPromise(): Promise<Serializ
cachedSerializedProjectGraphPromise =
processFilesAndCreateAndSerializeProjectGraph(plugins);
}
const graphWasRecomputed =
cachedSerializedProjectGraphPromise !== previousPromise;
const result = await cachedSerializedProjectGraphPromise;

if (wasScheduled) {
Expand All @@ -133,16 +137,22 @@ export async function getCachedSerializedProjectGraphPromise(): Promise<Serializ
: [result.error]
: [];

// Always write the daemon's current graph to disk to ensure disk cache
// stays in sync with the daemon's in-memory cache. This prevents issues
// where a non-daemon process writes a stale/errored cache that never
// gets overwritten by the daemon's valid graph.
// Write the daemon's current graph to disk to ensure disk cache stays
// in sync with the daemon's in-memory cache. This prevents issues where
// a non-daemon process writes a stale/errored cache that never gets
// overwritten by the daemon's valid graph.
//
// When the graph was just recomputed, always write so the new graph is
// persisted. When serving the same graph from memory, use
// writeCacheIfStale to skip the write unless an external process has
// modified the file since this process last wrote it.
if (
result.projectGraph &&
result.projectFileMapCache &&
result.sourceMaps
) {
writeCache(
const writeFn = graphWasRecomputed ? writeCache : writeCacheIfStale;
writeFn(
result.projectFileMapCache,
result.projectGraph,
result.sourceMaps,
Expand Down
43 changes: 42 additions & 1 deletion packages/nx/src/project-graph/nx-deps-cache.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { existsSync, mkdirSync, renameSync } from 'node:fs';
import { existsSync, mkdirSync, renameSync, statSync } from 'node:fs';
import { join } from 'path';
import { performance } from 'perf_hooks';
import { NxJsonConfiguration } from '../config/nx-json';
Expand Down Expand Up @@ -199,6 +199,13 @@ export function createProjectFileMapCache(
return newValue;
}

/**
* Tracks the mtime of the project graph cache file after the last successful
* writeCache() call. Used by writeCacheIfStale() to skip redundant writes
* when no external process has modified the cache file since the last write.
*/
let lastWrittenCacheMtimeMs: number | undefined;

export function writeCache(
cache: FileMapCache,
projectGraph: ProjectGraph,
Expand Down Expand Up @@ -246,6 +253,12 @@ export function writeCache(
);
}

try {
lastWrittenCacheMtimeMs = statSync(nxProjectGraph).mtimeMs;
} catch {
lastWrittenCacheMtimeMs = undefined;
}

done = true;
} catch (err: any) {
if (err instanceof Error) {
Expand All @@ -269,6 +282,34 @@ export function writeCache(
performance.measure('write cache', 'write cache:start', 'write cache:end');
}

/**
* Writes the cache only if the on-disk cache file has been modified since
* this process last wrote it (i.e. an external process overwrote it), or
* if this process has never written the cache.
*
* Use this instead of writeCache() on hot paths where the same graph may
* be served multiple times without changing (e.g. the daemon responding
* to repeated client requests).
*/
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);
}
Comment on lines +294 to +311
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical logic flaw: writeCacheIfStale will skip writing when the in-memory graph has changed but the file hasn't been modified externally.

Scenario:

  1. Process computes Graph A, writes it, sets lastWrittenCacheMtimeMs = T1
  2. Graph changes due to file modifications, daemon recomputes to Graph B
  3. writeCacheIfStale is called with new Graph B data
  4. Checks file mtime (still T1 since this process last wrote it)
  5. currentMtimeMs === lastWrittenCacheMtimeMs → returns without writing
  6. 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;
}
Suggested change
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

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


export function shouldRecomputeWholeGraph(
cache: FileMapCache,
packageJsonDeps: Record<string, string>,
Expand Down
Loading