-
Notifications
You must be signed in to change notification settings - Fork 437
fix(profiling): join v2 sampling thread on exit #13351
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
base: main
Are you sure you want to change the base?
Conversation
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 236 ± 2 ms. The average import time from base is: 237 ± 2 ms. The import time difference between this PR and base is: -1.42 ± 0.08 ms. Import time breakdownThe following import paths have shrunk:
|
BenchmarksBenchmark execution time: 2025-05-30 22:03:32 Comparing candidate commit d409710 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 501 metrics, 5 unstable metrics. scenario:iastaspectsospath-ospathnormcase_aspect
scenario:telemetryaddmetric-1-distribution-metric-1-times
|
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.
IIRC @P403n1x87 had some concern about whether this would slow down application exit too much? So we probably want his review on this.
@nsrip-dd Yeah, I'm also debating whether this is really necessary or not. I initially had thought whether this would fix some of the read-after-free problems we see from segfaults, but don't really have clear evidences. |
Just my two cents: IMHO it is the right thing to join this thread. |
Checklist
Reviewer Checklist