Skip to content
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
zkochan merged 4 commits into
mainfrom
bench/storedir-and-fsevents-allowbuild
Apr 24, 2026
Merged

bench(integrated-benchmark): level cold-install comparison on macOS and across platforms#264
zkochan merged 4 commits into
mainfrom
bench/storedir-and-fsevents-allowbuild

Conversation

@zkochan

@zkochan zkochan commented Apr 24, 2026

Copy link
Copy Markdown
Member

Three related bench-harness fixes discovered while digging into #263. No production code changes — only fixture files the harness embeds.

1. storeDir in pnpm-workspace.yaml (commit 7887d9e)

create_npmrc in the harness writes store-dir=<bench-dir>/store-dir into the fixture's .npmrc, and hyperfine --prepare wipes that path between iterations — the intent being a cold install per timed run.

Pacquet's .npmrc parser only reads the auth/registry subset though (see crates/npmrc/src/lib.rs:173-178, matching pnpm 11 which moved project-structural settings to pnpm-workspace.yaml). With no storeDir in the workspace yaml the bench silently fell through to pacquet's default — ~/Library/pnpm/store on macOS, ~/.local/share/pnpm/store on Linux.

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 then hit the warm-store cache-lookup path instead of the cold-install path, so the benchmark was measuring the per-file symlink_metadata pass from #260 rather than the fetch+extract+write pipeline it was designed to stress. Observed locally: hyperfine --prepare would happily wipe <bench-dir>/store-dir, pacquet would finish its install, and no store-dir ever appeared in the bench dir — the CAFS writes and hardlinks were all going through ~/Library/pnpm/store instead.

Adding storeDir: ./store-dir in pnpm-workspace.yaml makes 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 (commit 9481f35)

fsevents is a macOS-native optional dep of chokidar (several fixture entries pull it in). On a macOS developer machine it installs and pnpm emits ERR_PNPM_IGNORED_BUILDS: fsevents@1.2.13 because the fixture's .npmrc has ignore-scripts=true.

That error exits pnpm with status 1, hyperfine flags it via the harness's exit-code check, and just integrated-benchmark dies on the first warmup run. Adding fsevents: false to allowBuilds silences that specific package (matching what pnpm writes when you run pnpm approve-builds and 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 (commit d47e192)

Pacquet doesn't filter optional dependencies by os / cpu / libccreate_virtual_store iterates 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 supportedArchitectures to 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-lockfile creates store-dir/v11/.../fsevents/1.2.13/… and node_modules/.pnpm/node_modules/fsevents on macOS, and will do the equivalent on Linux.

Test plan

  • cargo build -p pacquet-integrated-benchmark.
  • Local just integrated-benchmark --verdaccio --with-pnpm HEAD main now completes on macOS and creates a populated <bench-dir>/store-dir/v11/index.db per iteration.
  • pnpm install --frozen-lockfile with the yaml in place downloads fsevents on macOS (visible in store-dir) — same behaviour expected on Linux.
  • CI Integrated-Benchmark — numbers will shift a bit for pnpm on Linux (now has to download & extract the extra cross-platform optionals). That's the point.

zkochan added 2 commits April 24, 2026 12:49
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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-dir to the benchmark fixture pnpm-workspace.yaml so pacquet/pnpm honor the bench-local store directory (instead of falling back to the user default store).
  • Add fsevents: false under allowBuilds to prevent pnpm macOS runs from exiting with ERR_PNPM_IGNORED_BUILDS when ignore-scripts=true.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Apr 24, 2026

Copy link
Copy Markdown

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00     19.5±0.52ms   223.0 KB/sec    1.00     19.5±0.51ms   222.8 KB/sec

…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 zkochan changed the title bench(integrated-benchmark): honour storeDir; silence fsevents warning bench(integrated-benchmark): level cold-install comparison on macOS and across platforms Apr 24, 2026
@zkochan zkochan merged commit 12d8a67 into main Apr 24, 2026
8 of 10 checks passed
@zkochan zkochan deleted the bench/storedir-and-fsevents-allowbuild branch April 24, 2026 12:14
zkochan added a commit that referenced this pull request Apr 24, 2026
# Conflicts:
#	crates/tarball/src/lib.rs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants