This repository was archived by the owner on May 14, 2026. It is now read-only.
bench(integrated-benchmark): level cold-install comparison on macOS and across platforms#264
Merged
Merged
Conversation
The fixture's `.npmrc` set `store-dir=<bench-dir>/store-dir`, but pacquet reads `.npmrc` for auth/registry only — project-structural settings like `storeDir` must come from `pnpm-workspace.yaml` (see `crates/npmrc/src/lib.rs:173-178`, matching pnpm 11). With no `storeDir` in the yaml the benchmark silently fell through to pacquet's default (`~/Library/pnpm/store` on macOS, `~/.local/share/pnpm/store` on Linux), so `hyperfine --prepare`'s `rm -rf <bench-dir>/store-dir` never actually wiped the store the install read from. On a developer machine whose default store already holds the fixture's 1352 snapshots — any prior `pnpm install` of a similar project — every bench iteration hit the warm-store cache-lookup path instead of the cold-install path. The measured wall time was dominated by the per-file `symlink_metadata` pass of #260, not by the fetch+extract+write pipeline the benchmark was designed to stress. Adding `storeDir: ./store-dir` makes the bench honour the prepared per-revision store. On CI this is a no-op (the default `~/.local/share/pnpm/store` is empty anyway on a fresh runner) — the fix prevents developer-local runs from producing meaningless numbers.
…acOS `fsevents` is a macOS-only optional dependency of `chokidar` (via several fixture entries). On Linux CI it isn't installed and pnpm never complains; on a developer macOS machine it installs and pnpm emits `ERR_PNPM_IGNORED_BUILDS: fsevents@1.2.13` because the fixture's `.npmrc` sets `ignore-scripts=true`. That error exits pnpm with status 1, which hyperfine flags via the benchmark harness's exit-code check and takes the whole `just integrated-benchmark` run down on the first warmup. Adding `fsevents: false` to `allowBuilds` silences the warning specifically for this package without re-enabling script execution elsewhere. Matches what pnpm emits when you run `pnpm approve-builds` and decline fsevents.
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the integrated benchmark fixture configuration so benchmark runs use a per-benchmark store directory (ensuring cold-install measurements on developer machines) and so pnpm runs don’t fail on macOS due to an fsevents ignored-builds warning.
Changes:
- Add
storeDir: ./store-dirto the benchmark fixturepnpm-workspace.yamlso pacquet/pnpm honor the bench-local store directory (instead of falling back to the user default store). - Add
fsevents: falseunderallowBuildsto prevent pnpm macOS runs from exiting withERR_PNPM_IGNORED_BUILDSwhenignore-scripts=true.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Micro-Benchmark ResultsLinux |
…ionals Pacquet doesn't filter optional dependencies by `os` / `cpu` / `libc` at the moment — `create_virtual_store` iterates every snapshot in the lockfile unconditionally, so `fsevents` (and every other platform-specific optional) ends up in the install payload regardless of runner. pnpm does filter, so on Linux CI it skips `fsevents` and friends entirely. That's a real payload difference: pnpm's install does strictly less tarball fetching / extraction / CAS writing than pacquet's on the same lockfile. Small in absolute terms for this fixture, but a hidden handicap in a benchmark whose whole point is to measure the fetch+extract+write pipeline. Set `supportedArchitectures` to cover every OS / CPU / libc combo we care about so pnpm pulls cross-platform optionals on every runner, matching pacquet's payload. The fix belongs on the pnpm side of the fixture because pacquet's filter-by-platform is a future code change with larger blast radius; bringing the two tools to the same install set in one-line config here is the lower-risk intermediate.
zkochan
added a commit
that referenced
this pull request
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related bench-harness fixes discovered while digging into #263. No production code changes — only fixture files the harness embeds.
1.
storeDirinpnpm-workspace.yaml(commit7887d9e)create_npmrcin the harness writesstore-dir=<bench-dir>/store-dirinto the fixture's.npmrc, andhyperfine --preparewipes that path between iterations — the intent being a cold install per timed run.Pacquet's
.npmrcparser only reads the auth/registry subset though (seecrates/npmrc/src/lib.rs:173-178, matching pnpm 11 which moved project-structural settings topnpm-workspace.yaml). With nostoreDirin the workspace yaml the bench silently fell through to pacquet's default —~/Library/pnpm/storeon macOS,~/.local/share/pnpm/storeon Linux.On a developer machine whose default store already holds the fixture's 1352 snapshots (any prior
pnpm installof a similar project), every bench iteration then hit the warm-store cache-lookup path instead of the cold-install path, so the benchmark was measuring the per-filesymlink_metadatapass from #260 rather than the fetch+extract+write pipeline it was designed to stress. Observed locally:hyperfine --preparewould happily wipe<bench-dir>/store-dir, pacquet would finish its install, and nostore-direver appeared in the bench dir — the CAFS writes and hardlinks were all going through~/Library/pnpm/storeinstead.Adding
storeDir: ./store-dirinpnpm-workspace.yamlmakes pacquet honour the prepared per-revision store. No-op on a fresh CI runner where the default store is empty anyway — the fix prevents developer-local runs from producing meaningless numbers.2.
allowBuilds: fsevents: false(commit9481f35)fseventsis a macOS-native optional dep ofchokidar(several fixture entries pull it in). On a macOS developer machine it installs and pnpm emitsERR_PNPM_IGNORED_BUILDS: fsevents@1.2.13because the fixture's.npmrchasignore-scripts=true.That error exits pnpm with status 1, hyperfine flags it via the harness's exit-code check, and
just integrated-benchmarkdies on the first warmup run. Addingfsevents: falsetoallowBuildssilences that specific package (matching what pnpm writes when you runpnpm approve-buildsand decline fsevents) without re-enabling scripts anywhere else. Also reached on Linux after the next commit pulls fsevents into the Linux install set.3.
supportedArchitectures(commitd47e192)Pacquet doesn't filter optional dependencies by
os/cpu/libc—create_virtual_storeiterates every snapshot in the lockfile unconditionally, so platform-specific optionals end up in its install payload regardless of runner.pnpm does filter, so on Linux CI it skips
fsevents(and every other darwin-only optional) entirely. That's a real payload difference: pnpm's install does strictly less tarball fetching / extraction / CAS writing than pacquet's on the same lockfile. Small in absolute terms for this fixture, but a hidden handicap in a benchmark whose whole point is to measure the fetch+extract+write pipeline.Setting
supportedArchitecturesto cover every OS / CPU / libc combo we care about makes pnpm pull cross-platform optionals on every runner, matching pacquet's payload. The fix belongs on the pnpm side of the fixture because pacquet's filter-by-platform is a future code change with larger blast radius — bringing the two tools to the same install set in one-line config here is the lower-risk intermediate.Verified locally: with the setting pulled into place,
pnpm install --frozen-lockfilecreatesstore-dir/v11/.../fsevents/1.2.13/…andnode_modules/.pnpm/node_modules/fseventson macOS, and will do the equivalent on Linux.Test plan
cargo build -p pacquet-integrated-benchmark.just integrated-benchmark --verdaccio --with-pnpm HEAD mainnow completes on macOS and creates a populated<bench-dir>/store-dir/v11/index.dbper iteration.pnpm install --frozen-lockfilewith the yaml in place downloads fsevents on macOS (visible instore-dir) — same behaviour expected on Linux.