test(anomaly): guard fixture search work both ways; make -bench . finish - #127
Merged
Merged
Conversation
Follow-up to PR #108, which raised the fixture rule's alternating shape from 72.96 to 103.0 gaps visited per pulse (+41%) without any guard seeing it: TestPeriodicSearchWorkIsBounded ran only the max and worst rules, and only checked the upper side. - The guard now also runs the fixture rule that BenchmarkPeriodicSearch uses for its "fixture/..." cases; measuredSteps is keyed by rule name and shape (the fixture and max rules share MinCoverage 0.6, so the old shape/MinCoverage key could not tell them apart). - measuredSteps holds master's actual totals (e.g. worst/jitter 6,242,089 -> 6,434,483 after #108). - The 25% margin is two-sided. More work fails as before; markedly fewer gaps visited fails too, with a message that it means lost search coverage and that a deliberate optimization updates measuredSteps. Reintroducing the seed's tolerance in chainRefined now fails here (fixture/alternating 73,761 < 104,271 - 25%). - The comment states that the counters count visited gaps only: removing the periodRange memo leaves them unchanged, and TestRangeMemoIsNeverStale covers the memo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h . finishes BenchmarkBytesPerStream ran its whole measurement (a new Detector, 4*n events, two forced GCs) inside the b.N loop with the timer stopped. The timed work was ~0, so the framework kept raising b.N towards 1e9 and repeated the measurement b.N times: `go test -run '^$' -bench . ./...` never finished (on 18a1326, aborted after 13.5 min, still in the first sub-benchmark; -timeout does not cover benchmarks). This predates #108. Each sub-benchmark now takes one heap measurement (bytesPerStream) outside the b.N loop, reuses it when the framework calls it again with a larger b.N, runs an empty b.N loop and reports B/stream with b.ReportMetric as before (ns/op is meaningless here). b.Loop is not used: the module targets Go 1.22. B/stream is unchanged (760.7, 1345, 747.9, 1332, 739.0, 1323 for 1k, 10k and 100k streams without and with periodicity, the same as master with -benchtime=1x); the full `-bench . -count=1` now completes in 63.5s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dborup
marked this pull request as ready for review
September 29, 2026 05:59
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
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.
Summary
Benchmark hygiene in
internal/anomaly, following the independent review of PR #108. Only*_test.gofiles change, and there are no production code changes. The library is still not wired into any binary.TestPeriodicSearchWorkIsBoundeddid not see the +41 % rise in visited gaps that fix(anomaly): periodic refinement, mixed-burst labels and censored-episode confidence #108 caused on the fixture rule's alternating shape (72.96 → 103.0 gaps per pulse). It also could not see a drop in work, which would mean lost search coverage.BenchmarkBytesPerStreamnever finished. It repeated a whole heap measurementb.Ntimes with the timer stopped, sogo test -run '^$' -bench . ./...did not complete. This predates fix(anomaly): periodic refinement, mixed-burst labels and censored-episode confidence #108.1. Work guard (
periodic_bench_test.go)Before:
maxPeriodicRule()andworstPeriodicRule(). The rule behindBenchmarkPeriodicSearch'sfixture/...cases (experimentalPeriodic()) was not covered.measuredStepswas keyed byshape/MinCoverage. The fixture and max rules both have MinCoverage 0.6, so that key could not tell them apart.Now:
fixture,maxandworst.measuredStepsis keyed byrule/shapeand holds master's actual totals (18a13264, 4 × 256 pulses per rule and shape).measuredSteps.periodRangememo, is not visible to them.TestRangeMemoIsNeverStalecovers the memo, and onlyBenchmarkPeriodicSearchmeasures the CPU.measuredSteps(master18a13264)periodic/0.6)alternating/0.6)jitter/0.6)missing/0.6)bursty/0.6)noise/0.6)periodic/1)alternating/1)jitter/1)missing/1)bursty/1)noise/1)Mutant evidence (run on scratch copies of this branch, full package suite):
periodic.gotol(seed)instead ofperiodRangefixture/alternating: 73761 steps in total, less than measured 104271 -25%: … lost search coverage …TestPeriodicRelativeJitterRefinesWithTheCandidateTolerance,TestRefinedCandidateReplacesOnlyWhenItWouldSignalperiodRangememo removed (memoRangealways recomputes)TestRangeMemoIsNeverStale:0 memo hits of 200000 queriesWith the old one-sided guard, the first mutant would have passed the guard: its steps fall from 104,271 to 73,761.
2.
BenchmarkBytesPerStream(bench_test.go)Before: the whole measurement sat inside
for it := 0; it < b.N; it++, with the timer stopped for the full iteration. The measurement is a new Detector, 4 × n events and two forced GCs. Because the timed work was about 0 ns, the framework kept raisingb.Ntowards 10⁹ and ran that many full measurements.Now:
bytesPerStream(n, periodic)) outside theb.Nloop, with the timer stopped, and reuses it when the framework calls the sub-benchmark again with a largerb.N.b.Nloop is empty, andB/streamis reported withb.ReportMetricas before.ns/opis meaningless for this benchmark, and the comment says so.b.Loop()is not used, because the module'sgo.modtargets Go 1.22.18a13264vs this branchgo test -run '^$' -bench . -count=1 ./...BytesPerStream/streams=1000/periodic=false(-timeout 10mdoes not cover benchmarks)go test -run '^$' -bench . -benchtime=1x -count=1 ./...Load during the runs: load average 3.4–3.9 on 12 cores.
B/stream is the same measurement as before, now taken exactly once per sub-benchmark:
-benchtime=1x)Test results (local, go1.26.0 darwin/arm64, in
internal/anomaly)gofmt -l .lists nothing, andgo vet ./...is clean.go test -race -count=1 ./...: ok (12.2 s).go test -run TestPeriodicSearchWorkIsBounded -count=3 -v ./...: 3/3 pass, and all 18 rule/shape totals are identical in all three runs (the counters are deterministic).go test -run '^$' -bench . -benchtime=1x -count=1 ./...: completes in 3.9 s.go test -run '^$' -bench . -count=1 ./...: completes in 63.5 s.Scope
internal/anomaly/periodic_bench_test.goandinternal/anomaly/bench_test.go, in one commit each.🤖 Generated with Claude Code