-
Notifications
You must be signed in to change notification settings - Fork 13
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
[PROF-11524] Minor tweak: Set profile start_time
after resetting
#987
base: main
Are you sure you want to change the base?
Conversation
**What does this PR do?** This PR applies a suggestion from #963 (comment) to set the `start_time` after obtaining the old profile. This will hopefully make it more clear that the `start_time` is used for the old profile, not the new one. **Motivation:** Make code more readable. **Additional Notes:** N/A **How to test the change?** Existing test coverage should be enough to validate this change (for instance, the Ruby profiler test suite explicitly tests the timestamps on profiles).
BenchmarksComparisonBenchmark execution time: 2025-03-31 12:34:43 Comparing candidate commit 80413e7 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 52 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #987 +/- ##
==========================================
- Coverage 72.73% 71.88% -0.86%
==========================================
Files 335 334 -1
Lines 51089 50291 -798
==========================================
- Hits 37161 36152 -1009
- Misses 13928 14139 +211
🚀 New features to boost your workflow:
|
What does this PR do?
This PR applies a suggestion from #963 (comment) to set the
start_time
after obtaining the old profile.This will hopefully make it more clear that the
start_time
is used for the old profile, not the new one.Motivation
Make code more readable.
Additional Notes
N/A
How to test the change?
Existing test coverage should be enough to validate this change (for instance, the Ruby profiler test suite explicitly tests the timestamps on profiles).