Skip to content

[Bug] Perf bench-count drift guard doesn't run when BenchCatalog.All changes, so /perf breaks after merge #1373

Open

Description

What happened?

tests/stress_perf/ci/PerfLib.Tests.ps1 has a drift guard: it parses BenchCatalog.All in tests/perf_bench/PerfBench.ControlModel/Benches/AllBenches.cs and asserts that the bench count equals the hand-maintained $script:MicroExpectedBenchCount in tests/stress_perf/ci/PerfLib.ps1 (currently 17). It's meant to stop the two from silently diverging.

The guard doesn't run when it matters. .github/workflows/perf-lib-tests.yml only triggers on changes under tests/stress_perf/ci/**. A PR that adds or removes a bench only touches tests/perf_bench/..., so the guard never runs, and the PR goes green with a mismatched count.

The mismatch only shows up after merge, the next time someone runs /perf. perf-compare.yml runs PerfLib.Tests.ps1 as its first step, the drift assertion fails, and the benchmark run aborts before it measures anything.

Expected: a PR that changes the number of entries in BenchCatalog.All without updating MicroExpectedBenchCount should fail CI on that PR.

Found while reviewing #1363, which adds six ItemsHost benches (IH1–IH6) to BenchCatalog.All (17 → 23). Its CI is fully green even though the count wasn't updated.

Suggested fix:

  • Add the bench catalog to the perf-lib-tests.yml path filters, for example tests/perf_bench/PerfBench.ControlModel/**, for both push and pull_request. The guard is pure source parsing on a Linux runner, so this costs seconds.
  • Optionally, have the drift assertion's failure message say exactly which literals to update. Today that's PerfLib.ps1:100, PerfLib.Tests.ps1:1137 and :1163; Run-PerfBenchmark.ps1 also mentions "17 benches" in its comments.
  • Optionally, make the job a required check, or fold it into an always-run job, so a path-filtered skip can't hide it.

Steps to reproduce

  1. On main, add one new SomeBench(), entry to BenchCatalog.All in tests/perf_bench/PerfBench.ControlModel/Benches/AllBenches.cs, without touching tests/stress_perf/ci/.
  2. Open a PR. Perf lib tests doesn't run, and CI is green.
  3. Run pwsh ./tests/stress_perf/ci/PerfLib.Tests.ps1 locally, or run /perf after merge. The drift guard: BenchCatalog.All declares 17 benches assertion fails.

Reactor version / commit

main @ 60b3a10. Also reproduced in the #1363 head, 92ea6ce74.

Platform

Both

.NET SDK version

10.0.402 (not SDK-dependent; this is a CI path-filter gap)

Windows version

N/A (the workflow runs on ubuntu-latest)

Windows App SDK version

N/A

Logs / stack trace

tests/stress_perf/ci/PerfLib.Tests.ps1:1163
  Assert-Equal 17 $catalogCount  'drift guard: BenchCatalog.All declares 17 benches (M1-M13 + OAlloc/OUpdate/C207 + M14)'
.github/workflows/perf-lib-tests.yml
  on.pull_request.paths: ['tests/stress_perf/ci/**', '.github/workflows/perf-lib-tests.yml']   # AllBenches.cs not included

Confirmation

  • I have searched existing issues and this isn't a duplicate.
  • This bug reproduces against the current main branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions