-
Notifications
You must be signed in to change notification settings - Fork 459
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
Update KeyValue to avoid copying non-static string slices #2089
Conversation
@@ -10,7 +10,7 @@ RAM: 64.0 GB | |||
| CreateTupleKeyValue | 671 ps | | |||
| CreateOtelKeyValueArray | 18.4 ns | | |||
| CreateOtelKeyValueArrayWithMixedValueTypes | 18.1 ns | | |||
| CreateOtelKeyValueArrayWithNonStaticValues | 90.1 ns | | |||
| CreateOtelKeyValueArrayWithNonStaticValues | 61.2 ns | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cijothomas The changes in this PR are helpful for this scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add metric e2e benchmarks too? the counter.add and friends
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2089 +/- ##
=====================================
Coverage 77.9% 77.9%
=====================================
Files 121 121
Lines 20969 20960 -9
=====================================
+ Hits 16335 16347 +12
+ Misses 4634 4613 -21 ☔ View full report in Codecov by Sentry. |
As usual, great work! Thanks for constantly pursuing to get the highest performance. From my knowledge of other OTel clients, none of them allow alloc-free recordings when the key/values are a slice/part of an existing string, so this is a great improvement! |
@utpilla can you resurrect this for 0.28 consideration? |
#2449 Closing inactive PR, we have issue to track this and linked to this PR. |
Related to #1642
Changes
Value
struct to avoid copying non-static string slicesBenchmarks
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes