Skip to content

Add focused benchmark for metric hotpath #1389

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.

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

Merged

Conversation

cijothomas
Copy link
Member

@cijothomas cijothomas commented Nov 22, 2023

I understand we have a lot of benchmarks, but I wanted to add something very focused, to help with PRs like #1379 (and its many follow ups).

Also added dedicated one for AttributeSet creation costs for various scenarios.

It okay to nuke these once the perf improvements are done, but I hope this is very useful to quickly measure PRs impact.
I was able to use this with #1379, and was able to quickly see the improvement.

@cijothomas cijothomas requested a review from a team November 22, 2023 02:42
@cijothomas
Copy link
Member Author

@KallDrexx I used this benchmarks against 1379. And here is my results (~16% boost). Could you check if you see similar?

Counter_Add_Sorted      time:   [839.97 ns 853.31 ns 869.54 ns]
                        change: [-16.163% -13.860% -11.672%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe

Counter_Add_Unsorted    time:   [832.49 ns 840.25 ns 848.39 ns]
                        change: [-13.431% -11.739% -10.075%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

@lalitb
Copy link
Member

lalitb commented Nov 22, 2023

The benchmark result of the test on my machine (~ 20% boost):

for main branch:

Counter_Add_Sorted      time:   [1.1975 µs 1.1985 µs 1.1996 µs]
Counter_Add_Unsorted    time:   [1.2020 µs 1.2041 µs 1.2076 µs]

for #1379 PR:

Counter_Add_Sorted      time:   [959.58 ns 963.14 ns 968.10 ns]
Counter_Add_Unsorted    time:   [965.16 ns 966.00 ns 966.89 ns]

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3229979) 57.2% compared to head (0ebe723) 57.2%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1389   +/-   ##
=====================================
  Coverage   57.2%   57.2%           
=====================================
  Files        146     146           
  Lines      18128   18128           
=====================================
+ Hits       10371   10372    +1     
+ Misses      7757    7756    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

LGTM.

@cijothomas
Copy link
Member Author

Merging now, as to leverage this for the pending PRs. Happy to address any additional feedbacks as followups.

@cijothomas cijothomas merged commit 04c863e into open-telemetry:main Nov 22, 2023
@cijothomas cijothomas deleted the cijothomas/metric_bench_smaller branch November 22, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants