From 87d7c12563c8e1786bce878bae78237c9315d467 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 2 Sep 2025 22:32:36 +0200 Subject: [PATCH] promql: Fix trigger for `HistogramStatsIterator` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #16702 introduced a regression because it was too strict in detecting the condition for using the `HistogramStatsIterator`. It essentially required the triggering function to be buried at least one level deep. `histogram_count(sum(rate(native_histogram_series[2m]))` would not trigger anymore, but `1*histogram_count(sum(rate(native_histogram_series[2m]))` would. Ironically, PR #16682 made the performance of the `HistogramStatsIterator` so much worse that _not_ using it was often better, but this has to be addressed in a separate commit. This commit reinstates the previous `HistogramStatsIterator` detection behavior, as PR #16702 intended to keep it. Relevant benchmark changes with this commit (i.e. old is without using `HistogramStatsIterator`, new is with `HistogramStatsIterator`): name old time/op new time/op delta NativeHistograms/histogram_count_with_short_rate_interval-16 802ms ± 3% 837ms ± 3% +4.42% (p=0.008 n=5+5) NativeHistograms/histogram_count_with_long_rate_interval-16 1.22s ± 3% 1.11s ± 1% -9.46% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_short_rate_interval-16 611ms ± 5% 751ms ± 6% +22.87% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_long_rate_interval-16 975ms ± 4% 1131ms ±11% +16.04% (p=0.008 n=5+5) name old alloc/op new alloc/op delta NativeHistograms/histogram_count_with_short_rate_interval-16 222MB ± 0% 531MB ± 0% +139.63% (p=0.008 n=5+5) NativeHistograms/histogram_count_with_long_rate_interval-16 323MB ± 0% 528MB ± 0% +63.81% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_short_rate_interval-16 179MB ± 0% 452MB ± 0% +153.07% (p=0.016 n=4+5) NativeHistogramsCustomBuckets/histogram_count_with_long_rate_interval-16 175MB ± 0% 452MB ± 0% +157.73% (p=0.016 n=4+5) name old allocs/op new allocs/op delta NativeHistograms/histogram_count_with_short_rate_interval-16 4.48M ± 0% 8.95M ± 0% +99.51% (p=0.008 n=5+5) NativeHistograms/histogram_count_with_long_rate_interval-16 5.02M ± 0% 8.84M ± 0% +75.89% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_short_rate_interval-16 3.00M ± 0% 5.96M ± 0% +98.93% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_long_rate_interval-16 2.89M ± 0% 5.86M ± 0% +102.69% (p=0.016 n=4+5) Signed-off-by: beorn7 --- promql/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promql/engine.go b/promql/engine.go index 9024df83da..2feb6c3c92 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -3891,7 +3891,7 @@ func detectHistogramStatsDecoding(expr parser.Expr) { return nil } - for i := len(path) - 1; i > 0; i-- { // Walk backwards up the path. + for i := len(path) - 1; i >= 0; i-- { // Walk backwards up the path. call, ok := path[i].(*parser.Call) if !ok { continue