Relabeling: small optimization to hashmod#13669
Conversation
… strconv.FormatInt() with better performance should be used. Signed-off-by: roger.wang <[email protected]>
|
even if it seems trivial to you, adding some benchmarks may help justify this. |
|
Agree with @machine424. |
There was a problem hiding this comment.
The difference is very minimal
goos: darwin
goarch: arm64
pkg: github.com/prometheus/prometheus/model/relabel
│ fmt.txt │ strconv.txt │
│ sec/op │ sec/op vs base │
Relabel/example-8 1.211µ ± 0% 1.210µ ± 0% ~ (p=0.350 n=100)
Relabel/kubernetes-8 5.432µ ± 0% 5.358µ ± 0% -1.35% (n=100)
geomean 2.564µ 2.546µ -0.70%
│ fmt.txt │ strconv.txt │
│ B/op │ B/op vs base │
Relabel/example-8 606.0 ± 0% 606.0 ± 0% 0.00% (p=0.010 n=100)
Relabel/kubernetes-8 1.417Ki ± 0% 1.417Ki ± 0% -0.03% (p=0.000 n=100)
geomean 937.7 937.6 -0.02%
│ fmt.txt │ strconv.txt │
│ allocs/op │ allocs/op vs base │
Relabel/example-8 19.00 ± 0% 19.00 ± 0% ~ (p=1.000 n=100) ¹
Relabel/kubernetes-8 36.00 ± 0% 36.00 ± 0% ~ (p=1.000 n=100) ¹
geomean 26.15 26.15 +0.00%
¹ all samples are equal
Number of test samples: 100
Run on MacBook Air M2 with 16GB of RAM and MacOS 14.4.1
|
Although I think using strconv would be idiomatic because it is more readable |
|
Maybe we should enforce this using a linter or sth. |
|
I created PR #14091, in which I made the same changes and enabled the |
As per your suggestions, i have created the pr #14092 which enables the linter and fixed all issues generated by it |
|
Thanks, @tesla59, for your responsiveness and efforts. However, I’m afraid @alexandear re-opened his PR first. He’s the author of the closed PR I linked to, where we discussed the reopening. You could help us review it if you want. |
Sure @machine424 I'll make sure to comment before working to avoid such conflicts |
grafana/mimir-prometheus@cf682ab Relevant PRs: prometheus/prometheus#14096 prometheus/prometheus#14068 prometheus/prometheus#13669 Signed-off-by: György Krajcsovits <[email protected]>
* Update mimir-prometheus to e5b85c151ba8 grafana/mimir-prometheus@e5b85c1 prometheus/prometheus#14103 prometheus/prometheus#14096 prometheus/prometheus#14068 prometheus/prometheus#13669 Signed-off-by: György Krajcsovits <[email protected]>
* Update mimir-prometheus to e5b85c151ba8 grafana/mimir-prometheus@e5b85c1 prometheus/prometheus#14103 prometheus/prometheus#14096 prometheus/prometheus#14068 prometheus/prometheus#13669 Signed-off-by: György Krajcsovits <[email protected]> (cherry picked from commit 307567a)
* Update mimir-prometheus to e5b85c151ba8 grafana/mimir-prometheus@e5b85c1 prometheus/prometheus#14103 prometheus/prometheus#14096 prometheus/prometheus#14068 prometheus/prometheus#13669 Signed-off-by: György Krajcsovits <[email protected]> (cherry picked from commit 307567a) Co-authored-by: George Krajcsovits <[email protected]>
* Update mimir-prometheus to e5b85c151ba8 grafana/mimir-prometheus@e5b85c1 prometheus/prometheus#14103 prometheus/prometheus#14096 prometheus/prometheus#14068 prometheus/prometheus#13669 Signed-off-by: György Krajcsovits <[email protected]>
bboreham
changed the title
6 participants
Code optimization: The relabel operation is used very frequently, and strconv.FormatInt() with better performance should be used.
Fixed #13668