From 815e507d67e6b0e45d33bdfbd6f9b698b5e185ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kh=E1=BA=A3i?= Date: Fri, 22 May 2026 13:03:51 +0700 Subject: [PATCH] ci(integrated-benchmark): scenarios without lockfiles (#11838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci(benchmark): run all six integrated-benchmark scenarios Wires `clean-install`, `full-resolution`, `peek`, and `gvs-warm` into `pacquet-integrated-benchmark.yml` so per-PR runs cover the same scenario set the manual `benchmark.yml` workflow already exercises via `benchmarks/bench.sh`. Requested for #11837, where the perf delta affects the resolution-bound scenarios (`firstInstall`, `withWarmCache`, `withWarmModules`, `updatedDependencies`) that the prior two-scenario set did not measure. Each scenario gets its own step with a 10 min hyperfine timeout (same rationale as the existing steps) and writes per-scenario report copies that the summary step concatenates into `SUMMARY.md`. * ci(benchmark): drop peek and gvs-warm scenarios Keep only the two new no-lockfile scenarios (`clean-install`, `full-resolution`) on top of the existing `frozen-lockfile` and `frozen-lockfile-hot-cache`. #11837's perf change is in the fresh-lockfile install path, which only runs when resolution runs — i.e., exactly the no-lockfile scenarios. `peek` mutates an existing lockfile and `gvs-warm` is a frozen-lockfile variant; neither exercises the affected path, and including them only costs per-PR CI wall time. * fix(bench): pin packages: ['.'] in synthesized pnpm-workspace.yaml The integrated-benchmark clones each pacquet revision's source tree into `/pacquet/`, which on the pnpm/pnpm monorepo includes upstream test fixtures like `workspace/project-manifest-reader/__fixtures__/invalid-package-json/package.json` — intentionally malformed JSON used to exercise pnpm's manifest reader. Without a `packages:` field, both pnpm's `findPackages.ts:28` and pacquet's `crates/workspace/src/projects.rs:128` default to `[".", "**"]`, so the fresh-resolve install path's `find_workspace_projects` walk descends into the cloned source tree and trips on the bad fixture: Error: pacquet_package_manifest::serialization_error × installing dependencies ╰─▶ expected `,` or `}` at line 3 column 3 The walk only runs on the fresh-lockfile branch (`install.rs:628-630`), which is why frozen-lockfile and frozen-lockfile-hot-cache stay green while clean-install and full-resolution fail every time. Pin `packages: ['.']` in the synthesized manifest so enumeration stays at the workspace root. The benchmark's installs are single-project, so this doesn't narrow anything the install actually needed to see. Fixtures supplied via `--fixture-dir` that already declare `packages:` keep their own value. * ci(benchmark): bump no-lockfile scenarios to 20 min Clean-install and full-resolution go through pacquet's fresh-resolve install path, which is currently ~3-5x slower than pnpm on the `alotta-files` fixture (pnpm/pnpm#11832). Hyperfine's default 1 warmup + 10 timed runs across three benchmark targets (pacquet@HEAD, pacquet@main, system pnpm) projects to ~13 min wallclock for these two scenarios, putting the previous 10 min cap right on the edge. Doubling to 20 min keeps the per-step timeout meaningful as a stuck- install detector without losing CI time when the bench is healthy. The frozen-lockfile steps stay at 10 min — they don't traverse the slower fresh-resolve path. * fix(bench): drop --no-frozen-lockfile from full-resolution scenario Pacquet doesn't expose `--no-frozen-lockfile` (only `--frozen-lockfile`, `--prefer-frozen-lockfile`, and `--no-prefer-frozen-lockfile`). Passing it makes clap reject the install: error: unexpected argument '--no-frozen-lockfile' found tip: a similar argument exists: '--frozen-lockfile' The flag was redundant for this scenario anyway: full-resolution starts every iteration with no lockfile on disk (init() skips the lockfile when `lockfile_enabled()` is false; cleanup removes it; `lockfile=false` in the synthesized npmrc/workspace prevents writing one). With no lockfile present the frozen path is unreachable regardless of the flag, so both tools take fresh resolution by definition. Fold full-resolution into clean-install's bare `install` arm. --------- Co-authored-by: Claude --- .../pacquet-integrated-benchmark.yml | 65 ++++++++++++++----- .../integrated-benchmark/src/cli_args.rs | 3 +- .../integrated-benchmark/src/work_env.rs | 11 ++++ .../src/workspace_manifest.rs | 16 +++++ 4 files changed, 75 insertions(+), 20 deletions(-) diff --git a/.github/workflows/pacquet-integrated-benchmark.yml b/.github/workflows/pacquet-integrated-benchmark.yml index fe83a1c95a..d2e95617ed 100644 --- a/.github/workflows/pacquet-integrated-benchmark.yml +++ b/.github/workflows/pacquet-integrated-benchmark.yml @@ -163,12 +163,29 @@ jobs: cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_FROZEN_LOCKFILE_HOT_CACHE.md cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_FROZEN_LOCKFILE_HOT_CACHE.json - # - name: 'Benchmark: Clean Install' - # shell: bash - # run: | - # just integrated-benchmark --scenario=clean-install --registry=verdaccio --with-pnpm pacquet@HEAD pacquet@main - # cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.md - # cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.json + - name: 'Benchmark: Clean Install' + shell: bash + # No-lockfile scenario: pacquet does fresh resolution against + # the registry, ~3-5x slower than pnpm on `alotta-files` + # (pnpm/pnpm#11832), so a single iteration runs longer than + # the frozen-lockfile steps. 20 min leaves headroom for the + # default hyperfine 1 warmup + 10 timed runs across all three + # benchmark targets without losing the per-command timeout + # safety net the other steps document. + timeout-minutes: 20 + run: | + just integrated-benchmark --scenario=clean-install --registry=verdaccio --with-pnpm pacquet@HEAD pacquet@main + cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.md + cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.json + + - name: 'Benchmark: Full Resolution' + shell: bash + # Same timeout rationale as the clean-install step above. + timeout-minutes: 20 + run: | + just integrated-benchmark --scenario=full-resolution --registry=verdaccio --with-pnpm pacquet@HEAD pacquet@main + cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_FULL_RESOLUTION.md + cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_FULL_RESOLUTION.json - name: Generate summary shell: bash @@ -199,18 +216,30 @@ jobs: echo '```' echo echo '' - # echo - # echo '### Scenario: Clean Install' - # echo - # cat bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.md - # echo - # echo '
BENCHMARK_REPORT.json' - # echo - # echo '```json' - # jq 'del(.results[].exit_codes)' bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.json - # echo '```' - # echo - # echo '
' + echo + echo '### Scenario: Clean Install' + echo + cat bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.md + echo + echo '
BENCHMARK_REPORT.json' + echo + echo '```json' + jq 'del(.results[].exit_codes)' bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.json + echo '```' + echo + echo '
' + echo + echo '### Scenario: Full Resolution' + echo + cat bench-work-env/BENCHMARK_REPORT_FULL_RESOLUTION.md + echo + echo '
BENCHMARK_REPORT.json' + echo + echo '```json' + jq 'del(.results[].exit_codes)' bench-work-env/BENCHMARK_REPORT_FULL_RESOLUTION.json + echo '```' + echo + echo '
' ) > bench-work-env/SUMMARY.md - name: Stage artifact contents diff --git a/pacquet/tasks/integrated-benchmark/src/cli_args.rs b/pacquet/tasks/integrated-benchmark/src/cli_args.rs index b56c9a3fdd..c28e25538f 100644 --- a/pacquet/tasks/integrated-benchmark/src/cli_args.rs +++ b/pacquet/tasks/integrated-benchmark/src/cli_args.rs @@ -128,12 +128,11 @@ impl BenchmarkScenario { /// element (`install` or `add`), followed by any flags. pub fn install_args(self) -> &'static [&'static str] { match self { - BenchmarkScenario::CleanInstall => &["install"], + BenchmarkScenario::CleanInstall | BenchmarkScenario::FullResolution => &["install"], BenchmarkScenario::FrozenLockfile | BenchmarkScenario::FrozenLockfileHotCache | BenchmarkScenario::GvsWarm => &["install", "--frozen-lockfile"], BenchmarkScenario::Peek => &["add", "is-odd"], - BenchmarkScenario::FullResolution => &["install", "--no-frozen-lockfile"], } } diff --git a/pacquet/tasks/integrated-benchmark/src/work_env.rs b/pacquet/tasks/integrated-benchmark/src/work_env.rs index 199cd7946b..0185a56bfe 100644 --- a/pacquet/tasks/integrated-benchmark/src/work_env.rs +++ b/pacquet/tasks/integrated-benchmark/src/work_env.rs @@ -521,6 +521,17 @@ fn create_pnpm_workspace( if manifest.store_dir.is_none() { manifest.store_dir = Some("./store-dir".to_string()); } + // Pin `packages: ['.']` when the fixture didn't set it. Without this + // the fresh-resolve install path's project walker + // (`find_workspace_projects`) defaults to `[".", "**"]` and recurses + // into the per-revision `/pacquet/` clone of pnpm/pnpm, + // tripping on the intentionally malformed test fixture at + // `workspace/project-manifest-reader/__fixtures__/invalid-package-json/package.json`. + // The benchmark's installs are always single-project, so restricting + // to the root is the right scope regardless of fixture. + if manifest.packages.is_none() { + manifest.packages = Some(vec![".".to_string()]); + } manifest.registry = Some(registry.to_string()); manifest.auto_install_peers = Some(true); manifest.ignore_scripts = Some(true); diff --git a/pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs b/pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs index b03cdb0feb..d1cbd2ff7c 100644 --- a/pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs +++ b/pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs @@ -25,6 +25,22 @@ pub struct MinimalWorkspaceManifest { pub ignore_scripts: Option, #[serde(skip_serializing_if = "Option::is_none")] pub lockfile: Option, + /// Workspace project globs. The benchmark forces this to `['.']` + /// (workspace root only) whenever the fixture doesn't set it, so + /// the fresh-resolve install path's + /// `find_workspace_projects` walk doesn't recurse into the + /// per-revision `/pacquet/` clone of `pnpm/pnpm` and + /// trip over upstream's intentionally malformed test fixtures + /// (e.g. `workspace/project-manifest-reader/__fixtures__/invalid-package-json/package.json`). + /// Both pnpm and pacquet default to `[".", "**"]` when the field + /// is absent — see pnpm's + /// [`findPackages.ts:28`](https://github.com/pnpm/pnpm/blob/9eb632bfbd/workspace/projects-reader/src/findPackages.ts#L28) + /// and pacquet's + /// [`projects.rs:128`](https://github.com/pnpm/pnpm/blob/9eb632bfbd/pacquet/crates/workspace/src/projects.rs#L128) — + /// which is why a synthesized manifest with no `packages:` field + /// pulls the source tree into the install's project enumeration. + #[serde(skip_serializing_if = "Option::is_none")] + pub packages: Option>, /// Mirrors pnpm's /// [`enableGlobalVirtualStore`](https://github.com/pnpm/pnpm/blob/94240bc046/config/reader/src/index.ts#L392-L394). /// Effective default is `false` for non-`--global` installs in