From f86711413fa475466e7bcb67b36f58691bb395f6 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Thu, 24 Sep 2026 14:22:23 +0100 Subject: [PATCH] Stop timing the one-cycle NormalizeRoute root fast path The advisory Benchmarks check kept failing unrelated pull requests on NormalizeRoute/root alone (#2228, #2242, #2245, #2250, and PR 1973 on 2026-09-08), and flagged it beside other results on #2230 and #2244. The "/" path is a length compare and a constant return. In the last 19 paired PR runs, the 11 on AMD EPYC 7763 measured root at exactly 2.18ns or 2.50ns on both sides, each within 0-1%. The two values are about one CPU cycle apart. In five of those runs base and candidate landed on different values, four times up (+14%) and once down (-13%), though none touched the normaliser. The earlier investigation found the benchmark loop identical and the normaliser differing only in relocated addresses. More samples cannot separate a stable one-cycle shift from a real change, and a looser threshold would blunt the gate for everything else. Batching root calls does not escape it. On an amd64 host a 64-call batch spread as widely in relative terms as a single call, because every call repeats the same few instructions. What the path must guarantee is that "/" and "" return before splitting. TestNormalizeRoute_RootFastPathDoesNotAllocate pins that deterministically: with the fast path removed, TestNormalizeRoute still passes, but the new test fails with one allocation per call. Real regressions stay visible. A paired run using the CI runner's own alternating recipe (10 samples, 100ms), with normalizeRoute made to do its work twice, flagged every remaining NormalizeRoute sub-benchmark at +89% to +107%, FullMiddlewarePath at +54%, and bytes and allocs at +100%. check-bench-regression.sh exited 1. The same tree compared with itself exited 0. --- internal/api/http_metrics_bench_test.go | 7 ++++++- internal/api/http_metrics_test.go | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/api/http_metrics_bench_test.go b/internal/api/http_metrics_bench_test.go index d11f00a32b..5d2e3fc813 100644 --- a/internal/api/http_metrics_bench_test.go +++ b/internal/api/http_metrics_bench_test.go @@ -13,6 +13,12 @@ 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. func BenchmarkNormalizeRoute(b *testing.B) { paths := []struct { name string @@ -23,7 +29,6 @@ func BenchmarkNormalizeRoute(b *testing.B) { {"with_uuid", "/api/nodes/550e8400-e29b-41d4-a716-446655440000"}, {"deep_path_truncated", "/api/v1/orgs/123/resources/456/metrics"}, {"with_query_params", "/api/metrics-store/history?resourceType=vm&resourceId=pve1:node1:100&metric=cpu&range=1h"}, - {"root", "/"}, {"metrics_history", "/api/metrics-store/history"}, } diff --git a/internal/api/http_metrics_test.go b/internal/api/http_metrics_test.go index 6370cc2838..3e6e98ab8a 100644 --- a/internal/api/http_metrics_test.go +++ b/internal/api/http_metrics_test.go @@ -246,6 +246,16 @@ func TestNormalizeRoute(t *testing.T) { } } +// The root and empty paths must return before splitting. Without the fast +// path, "/" still normalizes to "/" but allocates a split slice on every call. +func TestNormalizeRoute_RootFastPathDoesNotAllocate(t *testing.T) { + for _, path := range []string{"/", ""} { + if allocs := testing.AllocsPerRun(100, func() { _ = normalizeRoute(path) }); allocs != 0 { + t.Errorf("normalizeRoute(%q) allocated %v times per call, want 0", path, allocs) + } + } +} + // --------------------------------------------------------------------------- // recordAPIRequest integration tests — verify that the core recording // function correctly populates all three Prometheus metric vectors.