mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Stop timing the per-segment route normaliser (#2258)
The advisory Benchmarks check failed four pull requests on `NormalizeSegment` alone in the last 48 paired runs: #2217, #2225, #2252 and #2253. None of them touched the normaliser. Nine other runs moved the same rows down by more than 10%. The values are quantized (EPYC 7763: `short_name` 2.50 or 3.12ns, `uuid` about 29.6 or 40.5ns). Go aligns these functions to 32 bytes, so a change anywhere earlier in the binary moves them between the halves of a 64-byte block. This drops `BenchmarkNormalizeSegment`. Its guarantees move to deterministic tests: - Classification was already pinned by `TestNormalizeSegment`, `TestLooksLikeUUID` and `TestIsNumeric`. - The new `TestNormalizeSegment_DoesNotAllocate` pins zero allocations for the five former inputs. It stores each result in a package-level sink. Without the sink, an inlinable `normalizeSegment` could keep a short-segment copy on the stack, and the test would pass. Timing regressions still show in `BenchmarkNormalizeRoute`, because every path runs its segments through `normalizeSegment`. ### Proof These are paired runs of the CI runner's recipe on pulse-dev. The package list is narrowed to `./internal/api/` and the sample count raised to 30, because the shared host's load spread these rows by up to 25%. | Run | `check-bench-regression.sh` | Flagged | |---|---|---| | `looksLikeUUID` scans twice | exit 1 | only `NormalizeRoute/with_uuid`, +13.7% (p=0.005) | | `normalizeSegment` does its work twice | exit 1 | only `NormalizeRoute/with_uuid`, +21.9% (p=0.030) | | candidate vs copy of itself | exit 0 | none | | candidate vs itself + unrelated init-time function (32-byte shift) | exit 0 | none (`with_uuid` +9.2%, under threshold) | | old tree vs itself + same function | exit 1 | all five `NormalizeSegment` rows, +25% to +40% (p=0.000) | The deterministic test fails with one allocation per call when the pass-through return uses `strings.Clone` or `string([]byte(seg))`. The margin for a UUID-scan slowdown is thin. Doubling the scan reads as roughly +11% to +14% on `with_uuid`. Layout alone has moved that row by up to 9.2%, both on CI and in the probe above.
This commit is contained in:
@@ -14,11 +14,14 @@ import (
|
||||
// BenchmarkNormalizeRoute measures route normalization with representative
|
||||
// API paths of varying complexity.
|
||||
//
|
||||
// The "/" fast path is deliberately not timed here. Paired CI runs measured it
|
||||
// at either 2.18ns or 2.50ns, about one CPU cycle apart, and changes that never
|
||||
// touched this code flipped it both ways by 13-30%.
|
||||
// TestNormalizeRoute_RootFastPathDoesNotAllocate pins what matters about the
|
||||
// path instead.
|
||||
// Neither the "/" fast path nor normalizeSegment is timed on its own. They
|
||||
// take 2-40ns, and paired CI runs measured each at one of two values a CPU
|
||||
// cycle or two apart, per call or per byte of the UUID scan.
|
||||
// Changes that never touched this code flipped them both ways by 13-50%.
|
||||
// Every path below runs its segments through normalizeSegment, so a real
|
||||
// slowdown in segment classification still shows here, most in with_uuid.
|
||||
// TestNormalizeRoute_RootFastPathDoesNotAllocate and
|
||||
// TestNormalizeSegment_DoesNotAllocate pin the allocation guarantees.
|
||||
func BenchmarkNormalizeRoute(b *testing.B) {
|
||||
paths := []struct {
|
||||
name string
|
||||
@@ -135,30 +138,6 @@ func BenchmarkRecordAPIRequest_Parallel(b *testing.B) {
|
||||
})
|
||||
}
|
||||
|
||||
// BenchmarkNormalizeSegment measures per-segment normalization for the three
|
||||
// detection paths: numeric ID, UUID, and long token.
|
||||
func BenchmarkNormalizeSegment(b *testing.B) {
|
||||
segments := []struct {
|
||||
name string
|
||||
seg string
|
||||
}{
|
||||
{"numeric_id", "12345"},
|
||||
{"uuid", "550e8400-e29b-41d4-a716-446655440000"},
|
||||
{"long_token", "abcdefghijklmnopqrstuvwxyz1234567890abcdef"},
|
||||
{"short_name", "resources"},
|
||||
{"medium_name", "metrics-store"},
|
||||
}
|
||||
|
||||
for _, s := range segments {
|
||||
b.Run(s.name, func(b *testing.B) {
|
||||
b.ReportAllocs()
|
||||
for i := 0; i < b.N; i++ {
|
||||
_ = normalizeSegment(s.seg)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// BenchmarkFullMiddlewarePath simulates the complete per-request overhead:
|
||||
// normalizeRoute + recordAPIRequest, as executed by the ErrorHandler middleware.
|
||||
// Uses r.URL.Path (no query params) matching the real middleware call site.
|
||||
|
||||
@@ -192,6 +192,26 @@ func TestNormalizeSegment(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// normalizeSegment runs on every path segment of every request, and each class
|
||||
// returns a constant label or the segment itself. The sink makes the result
|
||||
// escape, as it does in normalizeRoute. Without it, an inlined normalizeSegment
|
||||
// could copy a short segment into a stack buffer and hide the allocation.
|
||||
var normalizeSegmentSink string
|
||||
|
||||
func TestNormalizeSegment_DoesNotAllocate(t *testing.T) {
|
||||
for _, seg := range []string{
|
||||
"12345",
|
||||
"550e8400-e29b-41d4-a716-446655440000",
|
||||
"abcdefghijklmnopqrstuvwxyz1234567890abcdef",
|
||||
"resources",
|
||||
"metrics-store",
|
||||
} {
|
||||
if allocs := testing.AllocsPerRun(100, func() { normalizeSegmentSink = normalizeSegment(seg) }); allocs != 0 {
|
||||
t.Errorf("normalizeSegment(%q) allocated %v times per call, want 0", seg, allocs)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeRoute(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Reference in New Issue
Block a user