From 95dfc47abce6616f55367f72e9ee683d2bc5ce94 Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Wed, 29 Apr 2026 15:12:00 -0400 Subject: [PATCH] promql: reduce per-step samples-read counting overhead Benchmarking the initial samples-read wiring with benchstat against origin/main showed geomean +1.88% sec/op across BenchmarkRangeQuery, driven by two hot-path hotspots: - rate(sparse[1m]) at 10000 steps regressed by up to ~39% because sparse metrics have many empty windows, and the per-step samples- read accounting ran totalHPointSize and countSamplesAfter before the existing "continue on empty window" check. - countSamplesAfter used sort.Search with a closure on the tiny slices typical of range-vector windows (6 points for [1m] at 10s resolution); the call overhead dominated the binary-search cost. Two changes address this: - Reorder the call/range-vector loop so the empty-window early continue runs before fullWindowCount / samplesReadCount are computed. fullWindowCount also now feeds samplesReadCount at step 0, eliminating the duplicate totalHPointSize call. - Rewrite countSamplesAfter as a backwards linear scan. The cutoff (maxt - interval) is near the end of the window, so only the last one or two points satisfy the predicate; backwards linear scan is O(k) for k matches and avoids the closure overhead. This also drops the unused sort import from promql/value.go. Also skip IncrementSamplesReadAtStep when the delta is zero so the per-step array write is elided on steps where the window did not advance (common for @-modifier and step-invariant queries). After these changes, the remaining overhead for range-vector queries is ~5-8% in the subset benches, accounted for by the counter increments themselves. Signed-off-by: Dan Cech --- promql/engine.go | 36 ++++++++++++++++++++---------------- promql/value.go | 22 +++++++++++----------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 5b54ad9649..b06a57be0d 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2279,12 +2279,13 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, counter++ } } - var samplesReadCount int64 + var maxt int64 // Evaluate the matrix selector for this series // for this step, but only if this is the 1st // iteration or no @ modifier has been used. - if ts == ev.startTimestamp || selVS.Timestamp == nil { - maxt := ts - offset + refetch := ts == ev.startTimestamp || selVS.Timestamp == nil + if refetch { + maxt = ts - offset mint := maxt - selRange switch { case selVS.Anchored: @@ -2294,16 +2295,9 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, maxt += durationMilliseconds(ev.lookbackDelta) } floats, histograms, startTimestamps = ev.matrixIterSlice(it, mint, maxt, floats, histograms, startTimestamps) - // For subquery-derived matrices, SamplesRead was already counted - // inside evalSubquery via MergeSamplesReadFromSubquery; skip here - // to avoid double-counting the storage I/O. - if !matrixFromSubquery { - if step == 0 { - samplesReadCount = int64(len(floats) + totalHPointSize(histograms)) - } else { - samplesReadCount = countSamplesAfter(floats, histograms, maxt-ev.interval) - } - } + } + if len(floats)+len(histograms) == 0 { + continue } // fullWindowCount reflects the matrix window consumed at this // step. With an @ modifier the same window is reused across all @@ -2311,8 +2305,18 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, // this after the conditional re-fetch handles both cases // uniformly. fullWindowCount := int64(len(floats) + totalHPointSize(histograms)) - if len(floats)+len(histograms) == 0 { - continue + // For subquery-derived matrices, SamplesRead was already counted + // inside evalSubquery via MergeSamplesReadFromSubquery; skip here + // to avoid double-counting the storage I/O. On step 0 the full + // window is new; on later steps only points past the previous + // step's cutoff are new. + var samplesReadCount int64 + if refetch && !matrixFromSubquery { + if step == 0 { + samplesReadCount = fullWindowCount + } else { + samplesReadCount = countSamplesAfter(floats, histograms, maxt-ev.interval) + } } inMatrix[0].Floats = floats inMatrix[0].Histograms = histograms @@ -2323,7 +2327,7 @@ func (ev *evaluator) eval(ctx context.Context, expr parser.Expr) (parser.Value, outVec, annos := call(vectorVals, inMatrix, e.Args, enh) warnings.Merge(annos) ev.samplesStats.IncrementSamplesAtStep(step, fullWindowCount) - if !matrixFromSubquery { + if samplesReadCount > 0 { ev.samplesStats.IncrementSamplesReadAtStep(step, samplesReadCount) } diff --git a/promql/value.go b/promql/value.go index 40f010152d..c35e525f3f 100644 --- a/promql/value.go +++ b/promql/value.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" "math" - "sort" "strconv" "strings" @@ -193,19 +192,20 @@ func totalHPointSize(histograms []HPoint) int { // countSamplesAfter returns the number of sample equivalents in floats and histograms // with timestamp strictly after cutoff. Float samples count as 1; histogram samples // count via HPoint.size. Used for range-vector sample stats to count only new points per step. +// +// Both slices are sorted by timestamp ascending. We scan backwards from the end +// because the call site uses cutoff = maxt - interval, which is typically close +// to the end of the window, so only the last one or two points satisfy the +// predicate. Backwards linear scan is O(k) for k matches and avoids the closure +// overhead that sort.Search imposes on these very small slices. func countSamplesAfter(floats []FPoint, histograms []HPoint, cutoff int64) int64 { var n int64 - - // Both slices are sorted by timestamp; binary-search for the first - // element after cutoff then count from there. - i := sort.Search(len(floats), func(i int) bool { return floats[i].T > cutoff }) - n += int64(len(floats) - i) - - j := sort.Search(len(histograms), func(j int) bool { return histograms[j].T > cutoff }) - for _, h := range histograms[j:] { - n += int64(h.size()) + for i := len(floats) - 1; i >= 0 && floats[i].T > cutoff; i-- { + n++ + } + for i := len(histograms) - 1; i >= 0 && histograms[i].T > cutoff; i-- { + n += int64(histograms[i].size()) } - return n }