Skip to content

Registry performance #4261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Nov 1, 2024
Merged

Registry performance #4261

merged 3 commits into from
Nov 1, 2024

Conversation

zalegrala
Copy link
Contributor

@zalegrala zalegrala commented Nov 1, 2024 •
edited
Loading

What this PR does:

Here we make some performance improvements to the counter and gauge in the registry. This is a follow-on to #4232 and #4244. Additionally, we modify the metric interface to skip passing of the externalLabels now that each implementation has been updated to take those labels at time of new.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@zalegrala
Copy link
Contributor Author 

❯ benchstat main.txt wip.txt
goos: linux
goarch: amd64
pkg: github.com/grafana/tempo/modules/generator
cpu: AMD Ryzen 7 PRO 7840U w/ Radeon 780M Graphics
           │  main.txt   │               wip.txt               │
           │   sec/op    │   sec/op     vs base                │
Collect-16   4.630µ ± 6%   3.790µ ± 9%  -18.14% (p=0.000 n=10)

           │  main.txt   │            wip.txt            │
           │ heap_in_use │ heap_in_use  vs base          │
Collect-16   17.59M ± 1%   17.71M ± 1%  ~ (p=0.109 n=10)

           │   main.txt   │               wip.txt               │
           │     B/op     │     B/op      vs base               │
Collect-16   13.54Ki ± 2%   12.65Ki ± 4%  -6.57% (p=0.000 n=10)

           │  main.txt   │               wip.txt                │
           │  allocs/op  │  allocs/op    vs base                │
Collect-16   30.50 ± 34%   18.00 ± 100%  -40.98% (p=0.037 n=10)

@zalegrala zalegrala marked this pull request as ready for review November 1, 2024 16:10
joe-elliott
joe-elliott approved these changes Nov 1, 2024
View reviewed changes
Copy link
Collaborator

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

love it!

@zalegrala zalegrala merged commit 8cb9728 into grafana:main Nov 1, 2024
16 checks passed
@zalegrala zalegrala deleted the registryPerformance branch November 1, 2024 18:37
@zalegrala zalegrala mentioned this pull request Nov 4, 2024
Merged
3 tasks
@electron0zero electron0zero mentioned this pull request Nov 28, 2024
Closed
@electron0zero electron0zero mentioned this pull request Jan 29, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joe-elliott joe-elliott joe-elliott approved these changes

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

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

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

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

@ie-pham ie-pham Awaiting requested review from ie-pham ie-pham is a code owner

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zalegrala @joe-elliott