Improve performance of WithLabelValues(...) #1360

colega · 2023-10-04T16:19:47Z

The slice with variadic arguments passed to MetricVec.WithLabelValues() was escaping to heap. This change fixes that by performing a copy of the slice before passing it to fmt.Errorf(), which is where the slice was escaping. This keeps the hot path without that allocation.

Meaningful benchmark results (skipping ~0 CPU and 0 alloc ones):

                                                               │    old.txt    │               new.txt                │
                                                               │    sec/op     │    sec/op     vs base                │
Counter/With_Label_Values-16                                     108.00n ±  6%   58.06n ±  1%  -46.24% (p=0.000 n=10)
Counter/With_Label_Values_and_Constraint-16                       174.5n ± 15%   136.8n ±  6%  -21.63% (p=0.000 n=10)
Counter/With_triple_Label_Values-16                               309.3n ± 12%   172.9n ±  1%  -44.08% (p=0.000 n=10)
Counter/With_triple_Label_Values_and_Constraint-16                591.5n ± 11%   418.9n ±  3%  -29.17% (p=0.000 n=10)
Counter/With_repeated_Label_Values-16                             212.9n ± 10%   116.8n ± 23%  -45.16% (p=0.000 n=10)
Counter/With_repeated_Label_Values_and_Constraint-16              406.2n ± 14%   275.1n ±  4%  -32.30% (p=0.000 n=10)
CounterWithLabelValuesConcurrent-16                               85.45n ±  2%   89.09n ±  2%   +4.26% (p=0.003 n=10)

                                                               │    old.txt     │                  new.txt                  │
                                                               │      B/op      │     B/op      vs base                     │
Counter/With_Label_Values-16                                       48.00 ± 0%        0.00 ± 0%  -100.00% (p=0.000 n=10)
Counter/With_Label_Values_and_Constraint-16                        96.00 ± 0%       48.00 ± 0%   -50.00% (p=0.000 n=10)
Counter/With_triple_Label_Values-16                                144.0 ± 0%         0.0 ± 0%  -100.00% (p=0.000 n=10)
Counter/With_triple_Label_Values_and_Constraint-16                 288.0 ± 0%       144.0 ± 0%   -50.00% (p=0.000 n=10)
Counter/With_repeated_Label_Values-16                              96.00 ± 0%        0.00 ± 0%  -100.00% (p=0.000 n=10)
Counter/With_repeated_Label_Values_and_Constraint-16              192.00 ± 0%       96.00 ± 0%   -50.00% (p=0.000 n=10)
CounterWithLabelValuesConcurrent-16                                48.00 ± 0%        0.00 ± 0%  -100.00% (p=0.000 n=10)

                                                               │   old.txt    │                 new.txt                 │
                                                               │  allocs/op   │ allocs/op   vs base                     │
Counter/With_Label_Values-16                                     1.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
Counter/With_Label_Values_and_Constraint-16                      2.000 ± 0%     1.000 ± 0%   -50.00% (p=0.000 n=10)
Counter/With_triple_Label_Values-16                              3.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
Counter/With_triple_Label_Values_and_Constraint-16               6.000 ± 0%     3.000 ± 0%   -50.00% (p=0.000 n=10)
Counter/With_repeated_Label_Values-16                            2.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
Counter/With_repeated_Label_Values_and_Constraint-16             4.000 ± 0%     2.000 ± 0%   -50.00% (p=0.000 n=10)
CounterWithLabelValuesConcurrent-16                              1.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)

I was surprised by the fact that BenchmarkCounterWithLabelValuesConcurrent didn't see a CPU improvement, regardless the fact that it removes an allocation. I ran it with -count=25 with the same result 🤷:

goos: linux
goarch: amd64
pkg: github.com/prometheus/client_golang/prometheus
cpu: 11th Gen Intel(R) Core(TM) i7-11700K @ 3.60GHz
                                    │ concurrent.old.txt │         concurrent.new.txt         │
                                    │       sec/op       │   sec/op     vs base               │
CounterWithLabelValuesConcurrent-16          84.46n ± 0%   88.59n ± 1%  +4.89% (p=0.000 n=25)

                                    │ concurrent.old.txt │         concurrent.new.txt         │
                                    │        B/op        │   B/op     vs base                 │
CounterWithLabelValuesConcurrent-16           48.00 ± 0%   0.00 ± 0%  -100.00% (p=0.000 n=25)

                                    │ concurrent.old.txt │         concurrent.new.txt          │
                                    │     allocs/op      │ allocs/op   vs base                 │
CounterWithLabelValuesConcurrent-16           1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=25)

It's worth noting that some of the runs in this benchmark take ~50ns, while others take ~85ns.

Thanks @bboreham on the pointer on how to remove the allocation here.

The slice with variadic arguments passed to MetricVec.WithLabelValues() was escaping to heap. This change fixes that by performing a copy of the slice before passing it to fmt.Errorf(), which is where the slice was escaping. This keeps the hot path without that allocation. Meaningful benchmark results (skipping ~0 CPU and 0 alloc ones): │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ Counter/With_Label_Values-16 108.00n ± 6% 58.06n ± 1% -46.24% (p=0.000 n=10) Counter/With_Label_Values_and_Constraint-16 174.5n ± 15% 136.8n ± 6% -21.63% (p=0.000 n=10) Counter/With_triple_Label_Values-16 309.3n ± 12% 172.9n ± 1% -44.08% (p=0.000 n=10) Counter/With_triple_Label_Values_and_Constraint-16 591.5n ± 11% 418.9n ± 3% -29.17% (p=0.000 n=10) Counter/With_repeated_Label_Values-16 212.9n ± 10% 116.8n ± 23% -45.16% (p=0.000 n=10) Counter/With_repeated_Label_Values_and_Constraint-16 406.2n ± 14% 275.1n ± 4% -32.30% (p=0.000 n=10) CounterWithLabelValuesConcurrent-16 85.45n ± 2% 89.09n ± 2% +4.26% (p=0.003 n=10) │ old.txt │ new.txt │ │ B/op │ B/op vs base │ Counter/With_Label_Values-16 48.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10) Counter/With_Label_Values_and_Constraint-16 96.00 ± 0% 48.00 ± 0% -50.00% (p=0.000 n=10) Counter/With_triple_Label_Values-16 144.0 ± 0% 0.0 ± 0% -100.00% (p=0.000 n=10) Counter/With_triple_Label_Values_and_Constraint-16 288.0 ± 0% 144.0 ± 0% -50.00% (p=0.000 n=10) Counter/With_repeated_Label_Values-16 96.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10) Counter/With_repeated_Label_Values_and_Constraint-16 192.00 ± 0% 96.00 ± 0% -50.00% (p=0.000 n=10) CounterWithLabelValuesConcurrent-16 48.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10) │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ Counter/With_Label_Values-16 1.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) Counter/With_Label_Values_and_Constraint-16 2.000 ± 0% 1.000 ± 0% -50.00% (p=0.000 n=10) Counter/With_triple_Label_Values-16 3.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) Counter/With_triple_Label_Values_and_Constraint-16 6.000 ± 0% 3.000 ± 0% -50.00% (p=0.000 n=10) Counter/With_repeated_Label_Values-16 2.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) Counter/With_repeated_Label_Values_and_Constraint-16 4.000 ± 0% 2.000 ± 0% -50.00% (p=0.000 n=10) CounterWithLabelValuesConcurrent-16 1.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>

bboreham

Excellent!

ArthurSens

I'm a bit inexperienced when it comes to go performance

Could you explain where exactly vals escapes? 🙂 and why if I'm not asking for too much :P

colega · 2023-10-05T07:42:19Z

There's a fmt.Errorf(msg string, args ...interface{}) call on the next line, and we're passing vals as one of those args. Since a slice can't fit into the "value" part of the interface tuple (type, value), it escapes to the heap and a pointer to that slice is stored instead.

A longer explanation after some googling about this topic: https://stackoverflow.com/questions/39492539/go-implicit-conversion-to-interface-does-memory-allocation

bboreham · 2023-10-05T09:40:40Z

In escape analysis, the compiler tries to prove that a pointer to data does not get stored somewhere such that it lives on past the return of the function it was created in.

Interfaces defeat this analysis, since the code that implements a method could come from anywhere in the program.
Escape analysis is conservative - if there is any chance of an escape, that's enough.
So, by and large, any time you see interface the value is going to escape.

Oleg's change makes a deliberate copy before the slice is passed as an interface{}, so now the compiler can prove that the original value does not escape.

ArthurSens · 2023-10-05T12:26:21Z

Thank you both very much for the explanation and contribution ❤️

ArthurSens · 2023-10-05T12:33:04Z

prometheus/labels.go

@@ -165,6 +165,8 @@ func validateValuesInLabels(labels Labels, expectedNumberOfValues int) error {

 func validateLabelValues(vals []string, expectedNumberOfValues int) error {
 	if len(vals) != expectedNumberOfValues {
+		// The call below makes vals escape, copy them to avoid that.
+		vals := append([]string(nil), vals...)


vals := vals

Would this have the same effect?

Interesting, I don't know. Try it!

I don't think it would work, as the slice still references the array that we've allocated on stack.

Note that we already receive a copy of vals when this function is called, so this doesn't change anything.

good point, thanks again!

colega mentioned this pull request Oct 4, 2023

Propose promextra.CounterVec / promextra.GaugeVec to reduce allocations grafana/mimir#6193

Closed

3 tasks

bboreham approved these changes Oct 4, 2023

View reviewed changes

ArthurSens approved these changes Oct 4, 2023

View reviewed changes

ArthurSens merged commit 51714a5 into prometheus:main Oct 5, 2023
8 checks passed

ArthurSens reviewed Oct 5, 2023

View reviewed changes

colega deleted the improve-performance-of-with-label-values branch December 4, 2023 17:17

ty-dc mentioned this pull request Jan 2, 2024

[dependencies] update prometheus/client_golang v1.16.0 to v1.18.0 kubernetes/kubernetes#122551

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance of WithLabelValues(...) #1360

Improve performance of WithLabelValues(...) #1360

colega commented Oct 4, 2023

bboreham left a comment

ArthurSens left a comment •

edited

Loading

colega commented Oct 5, 2023

bboreham commented Oct 5, 2023

ArthurSens commented Oct 5, 2023

ArthurSens Oct 5, 2023

bboreham Oct 5, 2023

colega Oct 5, 2023

colega Oct 5, 2023

ArthurSens Oct 5, 2023

Improve performance of WithLabelValues(...) #1360

Improve performance of WithLabelValues(...) #1360

Conversation

colega commented Oct 4, 2023

bboreham left a comment

Choose a reason for hiding this comment

ArthurSens left a comment • edited Loading

Choose a reason for hiding this comment

colega commented Oct 5, 2023

bboreham commented Oct 5, 2023

ArthurSens commented Oct 5, 2023

ArthurSens Oct 5, 2023

Choose a reason for hiding this comment

bboreham Oct 5, 2023

Choose a reason for hiding this comment

colega Oct 5, 2023

Choose a reason for hiding this comment

colega Oct 5, 2023

Choose a reason for hiding this comment

ArthurSens Oct 5, 2023

Choose a reason for hiding this comment

ArthurSens left a comment •

edited

Loading