mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
rcourtman
b4ea3761e8
The non-required **Benchmarks** check has failed on every non-superseded PR since about 15:00 UTC on 5 October. Examples are #2521, #2520, #2524, #2525, #2527, #2530 and #2532, each ending with `Paired base and candidate benchmarks (pull requests)` timed out after 25 minutes. The paired step runs 10 base and 10 candidate samples, so each second added per sample costs 20 seconds of step time. ### What made it slower Paired step duration across PR runs, with per-package seconds per sample taken from the `ok` lines in the job logs: | Period | Paired step | `internal/unifiedresources` | `internal/monitoring` | |---|---|---|---| | 24 Sep – 5 Oct 08:00 | 16–18 min | 7.9 s | 4.5 s | | after #2486 (08:11) | 20.5–20.9 min | 16.2 s | 5.0 s | | after #2518 (15:01) | killed at 25 min | 38.2 s | 9.9 s | - **#2518** added `BenchmarkRegistryProjectionSnapshot`. Its `cold` mode rebuilt the registry between `b.StopTimer` and `b.StartTimer` on every iteration. Go picks `b.N` from timed work only, so the empty-registry cases ran hundreds of thousands of untimed ingests. Per sample, `/0/cold/point-targets` took 10.3 s and `/0/cold/bulk-targets` 5.9 s at a 100 ms benchtime. That is the cause of the timeouts. - **#2486** added 46 owner-index and metadata sub-benchmarks, about +8 s per sample. That is legitimate coverage. #2518's `BenchmarkBroadcastMetricsSnapshot` and #2527 added about +6 s more. - `-count`, `-benchtime` and the package list did not change. ### Fix 1. **Root cause.** `cold` now invalidates the registry's derived state (canonical metadata, typed views, source-target index) instead of rebuilding the registry. The refresh never reads its own output, so this repeats a fresh ingest's first read: at 1000 VMs, 40011 vs 40014 allocs/op and about 5.09 MB/op, the same as before. Locally the 24-case family now runs in 4.9 s; before, its 6 cold cases alone took 33.8 s. 2. **Scope.** Paired PR runs now sample only packages whose in-module dependency closure (`go list -deps -test`) holds a changed file. A `_test.go` file selects only its own package. Changes to `go.mod`/`go.sum`, dirty or non-Git trees, or an unreadable graph select every package. Applied to today's failures: #2521, #2524 and #2525 sample nothing; #2530 samples `internal/api`; #2532 samples api and monitoring; #2520 samples 5 packages; #2527 samples 7. The metadata records `selection=` and `packages=`. 3. **Budget.** A module or shared-model change still samples nearly everything, about 24 minutes including the ~2.7 min warm-up. The step timeout is now a measured 30 minutes (job stays at 35). `PULSE_BENCH_BUDGET_SECONDS=1680` makes the script project its total after each round. Once the projection reaches 28 minutes it stops and prints each package's mean seconds per sample, which replaces an unexplained timeout. `TestPairedBenchmarksStopInsideTheirBudget` keeps budget < step timeout < job timeout. 4. **Store-backed chart benchmarks** (second commit). Go calls a benchmark function again for each `b.N` it tries. `HandleWorkloadCharts_StoreBacked` and `HandleWorkloadsSummaryCharts_StoreBacked` each seeded 84,000 points into a fresh SQLite store (about 0.6 s) to time a request of 0.2 ms or less, two or three times per sample. In CI that was 3.5 s and 3.6 s of internal/api's 11.5 s per sample. The timed loop now runs in a `range=4h` sub-benchmark, so the parent seeds once per run. Data, request and bytes/allocs per op are unchanged, and locally the pair falls from about 6 s to 1–1.5 s. The names gain `/range=4h`, so this PR's comparison reports them unmatched. Unpaired branch runs still sample every package. With the cold fix, their 15-minute step (13–14.2 min today) regains about 3 minutes of headroom. ### Verification - `scripts/tests/test-ci-benchmarks.sh` covers no-op, own-test-only, shared-dependency, unreadable-graph, dirty-tree, module-file, budget-exceeded, in-budget and invalid-budget cases. Five mutations of the selection and budget logic are each caught. - On the real package graph, selection maps `internal/models` to 7 packages, `internal/monitoring` to api+monitoring, an `internal/api` embed to api, and frontend or docs paths to none. - End to end with real Go on two clean clones (`origin/main` vs this change), only `./internal/unifiedresources/` was sampled. Its per-sample time went from 53.9 s to 26.7 s on a loaded Mac. - `go test ./scripts/installtests -run 'TestPairedBenchmarksStopInsideTheirBudget|TestBenchmarkQualificationRetainsProvenance'`, plus the script-reference and doc-drift suites. This PR's own Benchmarks run should sample only `internal/unifiedresources` and `internal/api`, because both changes are those packages' own test files.