Build: Discover runs all benchmarkable packages (fixes PR #148)#149
Merged
Build: Discover runs all benchmarkable packages (fixes PR #148)#149
Conversation
PR #148 (signal-safety refactor under packages/reactivity) exposed the per-package scoping bug in discover. The prior logic looked up tachometer configs inside the specific changed package and ran their matrix cells — but every tachometer config in this repo lives under packages/renderer, so a PR that only changes packages/reactivity resolved to an empty matrix and the bench workflow skipped silently. This is wrong in the base case: reactivity imports renderer consumes via workspace symlinks, so a signal-path change moves renderer's bench numbers. Same for templating → rendering, utils → everything, etc. The benches are end-to-end measurements that pull the whole framework through a workload; treating them as scoped-by-touched-package was premature. New logic: any trigger that reaches this workflow (push to main, or a pull_request that matched our path filter) runs every tachometer-ci*.json it finds under packages/*/bench/tachometer/. Path filter already restricts triggers to perf-relevant changes, so this doesn't over-fire. The push-branch and pull-request branches collapse to the same one-liner, which is a good sign. Ship.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⚪ No Meaningful Change for
|
| metric | Change |
|---|---|
add-20 |
-0.1% – +0.0% |
bulk-add-50 |
-1.2% – +1.4% |
clear-completed |
-1.0% – +0.5% |
create-1k |
-1.7% – +0.7% |
edit-start |
-1.0% – +1.4% |
filter-completed |
-1.6% – +1.3% |
remove-5-back |
-0.8% – +0.4% |
signal-computed-chain-10x30k |
-1.4% – +0.6% |
signal-reactive-fanout-500x1200 |
-1.7% – +0.3% |
signal-reactive-multi-read-5x80k |
-1.2% – +0.3% |
signal-reactive-set-index-300 |
-1.9% – +0.8% |
swap-rows |
-0.9% – +0.8% |
toggle-10 |
-0.5% – +0.3% |
🔍 Unsure (18)
Inconclusive (4)
The measured difference is small, and our sampling couldn't confidently place it above or below zero. Running more samples in a future run might settle these metrics.
| metric | Change | Expected Noise |
|---|---|---|
append-1k |
-3.5% – +0.1% | ±1% |
signal-reactive-list-filter-1000x300 |
-2.3% – +0.5% | ±1% |
signal-reactive-list-replace-1000x500 |
-3.0% – +2.5% | ±1% |
signal-reactive-push-1000x20 |
-3.2% – +0.3% | ±1% |
Too Fast to Measure Precisely (14)
On benches this short, system jitter (scheduling, GC, JIT) masks sub-4% changes; larger deltas still resolve cleanly.
| metric | Change | Test Time | Expected Noise |
|---|---|---|---|
clear |
-8.4% – +10.2% | ~12ms | ±13% |
edit-save |
-1.3% – +2.3% | ~20ms | ±8% |
remove-5-front |
-0.5% – +2.0% | ~85ms | ±2% |
remove-5-middle |
-2.0% – +0.3% | ~77ms | ±2% |
remove-first |
-2.7% – +1.0% | ~22ms | ±7% |
remove-last |
-13.5% – +8.1% | ~9ms | ±17% |
remove-middle |
-2.8% – +3.7% | ~15ms | ±10% |
select |
-5.3% – +6.7% | ~20ms | ±8% |
signal-reactive-set-property-by-id-100 |
-2.0% – -0.1% | ~115ms | ±1% |
toggle-all |
-0.9% – +2.5% | ~19ms | ±8% |
toggle-first |
-3.7% – +11.5% | ~6ms | ±25% |
toggle-last |
-3.2% – +13.8% | ~5ms | ±29% |
toggle-middle |
-7.3% – +6.6% | ~7ms | ±23% |
update-10th |
-4.5% – +3.4% | ~21ms | ±8% |
Sample size: 50 · Resolution floor: ±2% · Timeout: 3min · Wall-clock: 11m02s
jlukic
added a commit
that referenced
this pull request
Apr 15, 2026
Build: Discover runs all benchmarkable packages, not just touched ones PR #148 (signal-safety refactor under packages/reactivity) exposed the per-package scoping bug in discover. The prior logic looked up tachometer configs inside the specific changed package and ran their matrix cells — but every tachometer config in this repo lives under packages/renderer, so a PR that only changes packages/reactivity resolved to an empty matrix and the bench workflow skipped silently. This is wrong in the base case: reactivity imports renderer consumes via workspace symlinks, so a signal-path change moves renderer's bench numbers. Same for templating → rendering, utils → everything, etc. The benches are end-to-end measurements that pull the whole framework through a workload; treating them as scoped-by-touched-package was premature. New logic: any trigger that reaches this workflow (push to main, or a pull_request that matched our path filter) runs every tachometer-ci*.json it finds under packages/*/bench/tachometer/. Path filter already restricts triggers to perf-relevant changes, so this doesn't over-fire. The push-branch and pull-request branches collapse to the same one-liner, which is a good sign. Ship.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
PR #148 exposed a gap in the discover step: a change in `packages/reactivity/**` resolved to an empty bench matrix because the discover logic scoped matrix cells by per-package tachometer configs, and all our configs live under `packages/renderer/`. Net effect: reactivity changes silently skipped benchmarks, even though they obviously affect renderer perf via workspace imports.
Fix
Discover now always runs every benchmarkable package's configs (4 cells today: `tachometer-ci`, `tachometer-ci-signal`, `tachometer-ci-todo`, `tachometer-ci-todo-micro`). The path filter on the workflow trigger already restricts this to perf-relevant changes, so we don't over-fire.
Both the push-to-main branch and the pull_request branch collapse to the same logic — it was the same thing twice before, with different scoping bugs.
Impact on PR #148
Once this merges, #148 needs a rebase (or an empty commit) to re-trigger the bench workflow. The next run will properly fan out into the 4 bench cells and produce the first real bot-authored comment on a substantive perf PR.