Skip to content

Set up performance measurement for pks check - #53

Open
perryqh wants to merge 1 commit into
bump-rust-toolchain-1.97.1from
perf/measure-setup
Open

Set up performance measurement for pks check#53
perryqh wants to merge 1 commit into
bump-rust-toolchain-1.97.1from
perf/measure-setup

Conversation

@perryqh

@perryqh perryqh commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Groundwork for a series of performance changes to pks check. No behavior changes — this branch only makes the tool measurable, and fixes the release profile.

Note

Stacked on #52. The base is bump-rust-toolchain-1.97.1, so the diff above shows only the measurement work. GitHub will retarget this to main automatically once #52 merges.

Why

There was no way to answer "did that make it faster, and which phase moved?" without a lot of manual setup. dev/run_benchmarks.sh compares against packwerk but only reports a total, and assumed a specific directory layout.

What's here

1. [profile.release] was never configured. It sat at cargo defaults — lto = false, codegen-units = 16 — so cargo build --release, which is exactly what dev/run_benchmarks.sh measures, was less optimized than the shipped dist build. Now thin LTO and one codegen unit.

Measured on a 51,513-file application:

profile mean build time
default (before) 5.289 s ± 0.084 27 s
lto = "thin" 5.127 s ± 0.010 27 s
lto = "fat" 5.433 s ± 0.072 42 s

Fat LTO is slower to run and 55% slower to build, so thin it is — which also matches the existing dist profile. The numbers are recorded as a comment in Cargo.toml so this doesn't get re-litigated.

The variance drop matters as much as the mean: ±0.084 s → ±0.010 s. Several changes I want to measure next are worth 3–8%, which is not distinguishable from noise at the old variance.

2. dev/measure.sh — hyperfine mean plus a per-phase table derived from the --debug tracing already in the tool. No new instrumentation was needed for the phase breakdown; the tracing subscriber already timestamps every span.

PKS_APP=~/src/my_rails_app bash dev/measure.sh

3. Four trace points around the tail after the checkers finish. Dropping the reference vector, diffing package_todo.yml, writing output, and final teardown were previously one unexplained gap before process exit. They're now attributable — which immediately paid off: a gap I had estimated at 0.435 s turned out to be 0.120 s, and the "optimization" I was about to write for it would have been worth ~12 ms.

4. dev/run_benchmarks.sh — honors PKS_ROOT/PKS_BIN instead of hardcoding a sibling ../pks checkout, creates tmp/ if missing, and errors clearly if the binary isn't built. Also drops the single-file benchmark block, since that command is buggy and slated for removal and we shouldn't defend a number for it.

Sample output

==> [my-branch] hyperfine: pks check (warm cache, 2 warmup / 5 runs)
  Time (mean ± σ):      5.127 s ±  0.010 s
==> [my-branch] phase breakdown (single --debug run)
    0.559  Beginning directory walk
    2.876  Getting unresolved references (using cache if possible)
    0.665  Inferring constants from file name (using cache)
    0.671  Running checkers on resolved references
    ...

Verification

  • cargo test — 257 passing
  • cargo clippy --all-targets --all-features — clean
  • cargo fmt --all -- --check — clean
  • dev/measure.sh smoke-tested against tests/fixtures/simple_app, which caught a real bug: hyperfine aborts on a non-zero exit, and pks check exits 1 whenever it finds violations. Now passes --ignore-failure. Without that the script would have failed on essentially every real application.

🤖 Generated with Claude Code

Groundwork for a series of performance changes. No behavior changes.

- `[profile.release]` was left at cargo defaults (lto = false,
  codegen-units = 16), so `cargo build --release` -- what dev/run_benchmarks.sh
  measures -- was less optimized than the shipped `dist` build. Now thin LTO +
  one codegen unit. Measured on a 51k-file app: 5.289s -> 5.127s, and run
  variance drops from +/-0.084s to +/-0.010s.

  Fat LTO was measured too and is worse on both axes (5.433s, 42s build vs
  27s), so thin stays.

- Add `dev/measure.sh`: hyperfine mean plus a per-phase table derived from the
  `--debug` tracing already in the tool.

- Add trace points around the previously untraced tail after the checkers
  finish, so dropping the reference vector, diffing package_todo.yml, writing
  output, and final teardown are each attributable instead of appearing as one
  unexplained gap before process exit.

- dev/run_benchmarks.sh: honor PKS_ROOT/PKS_BIN instead of hardcoding a sibling
  ../pks checkout, and drop the single-file benchmark (that command is buggy and
  slated for removal, so we shouldn't track a number for it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@perryqh
perryqh force-pushed the perf/measure-setup branch from a2d3077 to 4008a93 Compare August 19, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant