Skip to content

fix(cache): fingerprint only whether node_modules/.modules.yaml exists - #850

Merged
wan9chi merged 1 commit into
mainfrom
fix/modules-yaml-existence-only
Oct 13, 2026
Merged

wan9chi merged 1 commit into
mainfrom
fix/modules-yaml-existence-only

Conversation

@wan9chi

@wan9chi wan9chi commented Oct 13, 2026

Copy link
Copy Markdown
Member

Motivation

#823 hashes pnpm's node_modules/.modules.yaml without prunedAt, storeDir, virtualStoreDir and linkedSkills. What's left protects little and still causes needless misses:

  • Every cached vite build and vitest task reads the file because Vite reads it while resolving its config (packages/vite/src/node/server/index.ts). Vite reads only virtualStoreDir, to add pnpm's global virtual store to the dev server's server.fs.allow, inside a try/catch for non-pnpm projects. virtualStoreDir is already left out of the hash, so the hash protects nothing Vite uses, and for vite build the file doesn't affect the output.
  • The other fields describe node_modules, which tasks already read. In pnpm 11 they are hoistedDependencies, hoistPattern, publicHoistPattern, hoistedLocations, nodeLinker, included, skipped, injectedDeps, layoutVersion, virtualStoreDirMaxLength, pendingBuilds, ignoredBuilds, allowBuilds, packageManager and virtualStoreOnly. When they change in a way that matters, the files a task reads in node_modules change too, and those reads are tracked.
  • Some of the hashed fields still vary across machines, so tasks missed with 'node_modules/.modules.yaml' modified:
    • packageManager: the version of the pnpm that installed
    • pendingBuilds: filled after pnpm install --ignore-scripts
    • virtualStoreOnly: set after pnpm fetch
    • skipped: differs between glibc and musl for platform packages under the same cache key
  • The list of fields to leave out has to track pnpm's format.

Changes

  • node_modules/.modules.yaml is now fingerprinted only by whether it exists, the same treatment fix(cache): fingerprint only whether files in node_modules/.bin exist #824 gives files in node_modules/.bin. A missing manifest still differs from an existing one, since tools detect pnpm by the file existing (Vite's try/catch, which-pm).
  • Trade-off: a task whose output depends on these fields without any file it reads in node_modules differing no longer misses. The examples fix(cache): leave noisy fields of pnpm's .modules.yaml out of its fingerprint #823 named are LavaMoat's allow-scripts check (pendingBuilds/ignoredBuilds), which-pm's pnpm version, and Nx's hoistedDependencies. These rarely run inside cached vp tasks, and in realistic cases node_modules differs too.
  • The JSON/YAML parsing, the canonical JSON writer and the fallback for unparsable content are removed, along with vt's serde_norway dependency. Both files are hashed as empty content without being read, which is the hash files in node_modules/.bin already had, so their cache entries stay valid.
  • The pnpm_modules_manifest e2e fixture now shows that changing the manifest's content keeps the cache hit and removing it misses.

🤖 Generated with Claude Code

Hashing a redacted copy of pnpm's modules manifest still missed the cache when fields that vary across machines changed, such as packageManager, pendingBuilds, virtualStoreOnly and skipped. Vite, which reads the file in every cached vite build and vitest task, uses only virtualStoreDir, which was already left out. The other fields describe node_modules, whose files tasks read and are tracked.

Fingerprint only whether the file exists, like files in node_modules/.bin, and remove the JSON/YAML parsing and the redaction machinery it needed, including vt's serde_norway dependency.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wan9chi
wan9chi force-pushed the fix/modules-yaml-existence-only branch from 2f156fb to f465a92 Compare October 13, 2026 07:26
@github-actions

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change  +0.42%  [ -4.22% ..  +5.12%]  overhead   +50.30%
dynamic/access             change  -0.44%  [ -1.96% ..  +1.25%]  overhead   +10.42%
dynamic/access-relative    change  +0.77%  [ -1.08% .. +16.45%]  overhead   +62.90%
dynamic/access-contended   change  +4.84%  [-12.03% .. +14.43%]  overhead   +24.13%
static/launch              change  +2.87%  [ -7.40% .. +11.57%]  overhead  +186.86%
static/access              change  +2.64%  [-10.05% .. +20.09%]  overhead  +895.06%
static/access-relative     change  -0.97%  [ -4.73% ..  +3.08%]  overhead +1377.39%
static/access-contended    change  +0.09%  [ -9.38% .. +12.48%]  overhead +2092.14%

macos

dynamic/launch             change  -0.43%  [ -2.87% ..  +2.98%]  overhead  +220.86%
dynamic/access             change  -0.40%  [ -4.14% ..  +2.81%]  overhead    +5.88%
dynamic/access-relative    change  -0.22%  [ -2.19% ..  +1.76%]  overhead  +272.91%
dynamic/access-contended   change  +0.28%  [ -3.05% ..  +2.38%]  overhead    +4.03%

windows

dynamic/launch             change  +1.34%  [ -7.35% ..  +7.85%]  overhead   +28.92%
dynamic/access             change  -0.18%  [ -3.87% ..  +4.46%]  overhead    +2.19%
dynamic/access-relative    change  +0.35%  [ -3.66% ..  +8.45%]  overhead    +2.14%
dynamic/access-contended   change  -0.80%  [ -3.70% ..  +3.46%]  overhead    +1.90%

@wan9chi
wan9chi merged commit 1ba1e46 into main Oct 13, 2026
32 of 34 checks passed
@wan9chi
wan9chi deleted the fix/modules-yaml-existence-only branch October 13, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant