Skip to content

promql: avoid unnecessary Metric.Get() calls in functions.go#17676

Merged
bboreham merged 3 commits intoprometheus:mainfrom
vpranckaitis:avoid_unnecesary_extraction_of_metric_name
Jan 8, 2026
Merged

promql: avoid unnecessary Metric.Get() calls in functions.go#17676
bboreham merged 3 commits intoprometheus:mainfrom
vpranckaitis:avoid_unnecesary_extraction_of_metric_name

Conversation

@vpranckaitis
Copy link
Contributor

@vpranckaitis vpranckaitis commented Dec 12, 2025

Moved some Metric.Get() calls in PromQL functions to avoid unnecessary label extraction. With stringlabels that is a non-trivial amount of work, as shown by CPU profiles. In many cases, this work was done to extract metric name, and was only used if annotations were emitted.

In the same go I also replaced labels.MetricName with model.MetricNameLabel, since the former was deprecated.

image

Which issue(s) does the PR fix:

Does this PR introduce a user-facing change?

[PERF] PromQL: Avoid unnecessary label extraction in PromQL functions

@vpranckaitis
[PERF] Avoid unnecessary Metric.Get() calls in functions.go
ed164d2 
Signed-off-by: Vilius Pranckaitis <vpranckaitis@gmail.com>
vpranckaitis
vpranckaitis commented Dec 12, 2025
View reviewed changes
@bboreham
Copy link
Member

bboreham commented Dec 12, 2025

Please show benchmark results, as requested in the PR template.

bboreham
bboreham reviewed Dec 12, 2025
View reviewed changes
Copy link
Member

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this; I think it's a good idea.

I would rather defer evaluation as far as possible, e.g. into histogramRate, and add a small helper function to shorten the code where used. Then don't create a new variable metricName anywhere it would only be used once.

@vpranckaitis
Copy link
Contributor Author

vpranckaitis commented Dec 12, 2025

Please show benchmark results, as requested in the PR template.

Did not write a separate benchmark, but used the exisitng BenchmarkRangeQuery() to show the improvement. To cut out the noise, I removed all the test cases except these two:

cases := []benchCase{
    // Simple rate.
    {
	    expr: "rate(a_X[1m])",
    },
    {
	    expr:  "rate(a_X[1m])",
	    steps: 10000,
    },
}

The benchmark results show 1–6% reduction in runtime, and practically no change in allocations.

goos: darwin
goarch: arm64
pkg: github.com/prometheus/prometheus/promql
cpu: Apple M1 Pro
                                                   │   old.txt   │              new.txt               │
                                                   │   sec/op    │   sec/op     vs base               │
RangeQuery/expr=rate(a_one[1m]),steps=1-10           9.016µ ± 1%   8.819µ ± 1%  -2.19% (p=0.000 n=10)
RangeQuery/expr=rate(a_one[1m]),steps=1000-10        84.93µ ± 0%   79.18µ ± 1%  -6.77% (p=0.000 n=10)
RangeQuery/expr=rate(a_hundred[1m]),steps=1-10       216.7µ ± 0%   214.4µ ± 1%  -1.06% (p=0.002 n=10)
RangeQuery/expr=rate(a_hundred[1m]),steps=1000-10    8.172m ± 1%   7.763m ± 2%  -5.01% (p=0.000 n=10)
RangeQuery/expr=rate(a_one[1m]),steps=10000-10       824.9µ ± 0%   779.1µ ± 1%  -5.55% (p=0.000 n=10)
RangeQuery/expr=rate(a_hundred[1m]),steps=10000-10   85.89m ± 1%   80.89m ± 1%  -5.82% (p=0.000 n=10)
geomean                                              676.7µ        646.8µ       -4.42%

                                                   │   old.txt    │               new.txt               │
                                                   │     B/op     │     B/op      vs base               │
RangeQuery/expr=rate(a_one[1m]),steps=1-10           7.883Ki ± 0%   7.883Ki ± 0%       ~ (p=0.752 n=10)
RangeQuery/expr=rate(a_one[1m]),steps=1000-10        11.58Ki ± 0%   11.58Ki ± 0%       ~ (p=0.631 n=10)
RangeQuery/expr=rate(a_hundred[1m]),steps=1-10       68.97Ki ± 0%   68.97Ki ± 0%       ~ (p=0.670 n=10)
RangeQuery/expr=rate(a_hundred[1m]),steps=1000-10    309.4Ki ± 0%   309.5Ki ± 0%       ~ (p=0.781 n=10)
RangeQuery/expr=rate(a_one[1m]),steps=10000-10       52.84Ki ± 1%   52.83Ki ± 1%       ~ (p=0.469 n=10)
RangeQuery/expr=rate(a_hundred[1m]),steps=10000-10   2.585Mi ± 0%   2.581Mi ± 1%  -0.14% (p=0.009 n=10)
geomean                                              80.52Ki        80.50Ki       -0.03%

                                                   │   old.txt   │               new.txt                │
                                                   │  allocs/op  │  allocs/op   vs base                 │
RangeQuery/expr=rate(a_one[1m]),steps=1-10            136.0 ± 0%    136.0 ± 0%       ~ (p=1.000 n=10) ¹
RangeQuery/expr=rate(a_one[1m]),steps=1000-10         165.0 ± 0%    165.0 ± 0%       ~ (p=1.000 n=10) ¹
RangeQuery/expr=rate(a_hundred[1m]),steps=1-10       1.136k ± 0%   1.136k ± 0%       ~ (p=1.000 n=10) ¹
RangeQuery/expr=rate(a_hundred[1m]),steps=1000-10    3.577k ± 0%   3.577k ± 0%       ~ (p=0.265 n=10)
RangeQuery/expr=rate(a_one[1m]),steps=10000-10        457.0 ± 0%    457.0 ± 0%       ~ (p=1.000 n=10) ¹
RangeQuery/expr=rate(a_hundred[1m]),steps=10000-10   27.51k ± 0%   27.51k ± 0%       ~ (p=0.524 n=10)
geomean                                              1.023k        1.023k       -0.00%
¹ all samples are equal

@vpranckaitis
refactor
0bfc6a0 
Signed-off-by: Vilius Pranckaitis <vpranckaitis@gmail.com>
@vpranckaitis
Copy link
Contributor Author

vpranckaitis commented Dec 15, 2025

Thanks for this; I think it's a good idea.

I would rather defer evaluation as far as possible, e.g. into histogramRate, and add a small helper function to shorten the code where used. Then don't create a new variable metricName anywhere it would only be used once.

@bboreham I've refactored the code to use getMetricName() helper function. I considered to name it getName() so that in majority of places where it's used it would look like getName(samples.Metric). The latter is shorter, but still should convey that the metric name is being retrieved. However, I decided to go with a more conservative option first.

Also, pushed the metric name extraction further down into histogramRate() function.

linasm
linasm approved these changes Jan 6, 2026
View reviewed changes
Copy link
Contributor

@linasm linasm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one nit.

bboreham
bboreham approved these changes Jan 6, 2026
View reviewed changes
Copy link
Member

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

@bboreham bboreham merged commit 6a81e44 into prometheus:main Jan 8, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bboreham bboreham bboreham approved these changes

@roidelapluie roidelapluie Awaiting requested review from roidelapluie roidelapluie is a code owner

+1 more reviewer

@linasm linasm linasm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vpranckaitis @bboreham @linasm