Skip to content

Commit 0a3cf22

Browse files
committed
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.
1 parent cc52ae2 commit 0a3cf22

1 file changed

Lines changed: 15 additions & 36 deletions

File tree

.github/workflows/benchmarks.yml

Lines changed: 15 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -48,43 +48,22 @@ jobs:
4848
- name: Build matrix
4949
id: matrix
5050
run: |
51-
# Push to main: archive every benchmarkable package — we need the
52-
# full absolute-CI snapshot for bench-history regardless of what
53-
# changed in this commit.
54-
if [ '${{ github.event_name }}' = 'push' ]; then
55-
changed=$(find packages -name 'tachometer-ci*.json' -path '*/bench/tachometer/*' \
56-
| cut -d/ -f2 \
57-
| sort -u)
58-
else
59-
base=origin/${{ github.event.pull_request.base.ref }}
60-
61-
# If a benchmark workflow or bench harness changed, we need to
62-
# validate CI itself — run against every benchmarked package.
63-
ci_changed=$(git diff --name-only "$base"...HEAD \
64-
| grep -E '^(\.github/workflows/benchmarks.*\.yml|packages/[^/]+/bench/tachometer/)' \
65-
| head -1)
66-
67-
if [ -n "$ci_changed" ]; then
68-
changed=$(find packages -name 'tachometer-ci*.json' -path '*/bench/tachometer/*' \
69-
| cut -d/ -f2 \
70-
| sort -u)
71-
else
72-
changed=$(git diff --name-only "$base"...HEAD \
73-
| grep '^packages/' \
74-
| cut -d/ -f2 \
75-
| sort -u)
76-
fi
77-
fi
78-
79-
# For each changed package, emit one entry per tachometer-ci*.json
51+
# Any trigger that reaches this workflow — push to main, or a
52+
# pull_request whose diff matched our path filter — runs every
53+
# benchmarkable package's configs. Tachometer configs only live
54+
# under `packages/renderer/` today, but a change in any other
55+
# package (reactivity, templating, etc.) can absolutely move
56+
# renderer's bench numbers via workspace imports. Per-package
57+
# scoping of the matrix was premature: the benches are end-to-end
58+
# measurements that pull the whole framework through; treat the
59+
# set of benchmarkable packages as a single target suite.
8060
entries=()
81-
for pkg in $changed; do
82-
while IFS= read -r config; do
83-
[ -z "$config" ] && continue
84-
name=$(basename "$config" .json)
85-
entries+=("{\"config\":\"$config\",\"name\":\"$name\",\"package\":\"$pkg\"}")
86-
done < <(find "packages/$pkg" -name 'tachometer-ci*.json' -path '*/bench/tachometer/*' 2>/dev/null | sort)
87-
done
61+
while IFS= read -r config; do
62+
[ -z "$config" ] && continue
63+
pkg=$(echo "$config" | cut -d/ -f2)
64+
name=$(basename "$config" .json)
65+
entries+=("{\"config\":\"$config\",\"name\":\"$name\",\"package\":\"$pkg\"}")
66+
done < <(find packages -name 'tachometer-ci*.json' -path '*/bench/tachometer/*' 2>/dev/null | sort)
8867
8968
if [ ${#entries[@]} -eq 0 ]; then
9069
matrix="[]"

0 commit comments

Comments
 (0)