-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[BUG FIX] Profile Timings incorrect for concurrent segments #18540
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
Conversation
Signed-off-by: TJ Neuenfeldt <[email protected]>
❌ Gradle check result for b537ce0: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <[email protected]>
❌ Gradle check result for ad563e7: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: TJ Neuenfeldt <[email protected]>
❕ Gradle check result for 89647fe: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18540 +/- ##
============================================
- Coverage 72.75% 72.75% -0.01%
- Complexity 68087 68142 +55
============================================
Files 5537 5537
Lines 313232 313233 +1
Branches 45458 45460 +2
============================================
- Hits 227905 227902 -3
- Misses 66758 66811 +53
+ Partials 18569 18520 -49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
@jainankitk Can you take a look as well?
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.
LGTM!
…ch-project#18540) --------- Signed-off-by: TJ Neuenfeldt <[email protected]>Signed-off-by: TJ Neuenfeldt <[email protected]>
…ch-project#18540) --------- Signed-off-by: TJ Neuenfeldt <[email protected]>
Description
As #18534 discusses, the profiler with concurrent segment search enabled had a bug where the start time was displayed in the output as the duration of a timer. This was due to it modifying the sliceStartTime for a timing type and when it was 0, that was the minimum so no other slices could overwrite it. Now, it correctly ignores slices where it didn't even time that TimingType (count = 0), so that the duration is correct. I reran the commands that exposed the bug and now the output is normal.
Related Issues
Resolves #18534
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.