-
Notifications
You must be signed in to change notification settings - Fork 821
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
perf(propagator-jaeger): improve deserializeSpanContext performance #3541
perf(propagator-jaeger): improve deserializeSpanContext performance #3541
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3541 +/- ##
==========================================
+ Coverage 93.96% 93.99% +0.02%
==========================================
Files 260 260
Lines 7805 7804 -1
Branches 1622 1621 -1
==========================================
+ Hits 7334 7335 +1
+ Misses 471 469 -2
|
82dd086
to
e6b1df5
Compare
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.
Seems fine but can you show benchmark results? AFAIK regex literals are not recompiled on each use by optimized runtimes like v8
|
ddad538
to
071a32c
Compare
@dyladan @legendecas i'm sorry, it's very hard to merge it without your approve |
Yeah I'll merge it there were just other things pulling me away yesterday |
Which problem is this PR solving?
Function
deserializeSpanContext()
atopentelemetry-propagator-jaeger
is not optimized.Short description of the changes
re.test(str)
works much faster thanstr.match(re)
because it doesn't store captured resultsflags.match(/^[0-9a-f]{1,2}$/i)
) on everyextract()
is slow. Compile it once much better.Type of change
How Has This Been Tested?
existing unit-tests is enough
Checklist: