Investigate and reduce benchmark changes for mesh timestamps #6549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6549 +/- ##
=======================================
Coverage 89.89% 89.89%
=======================================
Files 90 90
Lines 24138 24142 +4
Branches 4492 4493 +1
=======================================
+ Hits 21699 21703 +4
Misses 1679 1679
Partials 760 760 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
⏱️ Performance Benchmark Report: 371284cPerformance shiftsFull benchmark resultsGenerated by GHA run |
⏱️ Performance Benchmark Report: 85ef2caPerformance shiftsFull benchmark resultsGenerated by GHA run |
|
These benchmarks I think redeem the previous slow-downs, but used dodgy logic to do so and caused test failures. Note that we did expect adding timestamps would introduce slowdowns, but not overwhelmingly significant ones. |
⏱️ Performance Benchmark Report: 217571ePerformance shiftsFull benchmark resultsGenerated by GHA run |
|
So, this latest solution still doesn't seem to be cutting it, which suggests most of the slowdowns are that they are accessing metadata repeatedly, and therefore constantly refreshing points and bounds, which is comparatively slow. I think the only other solution I can think of would be to create a second timestamp, one for metadata, one for points and bounds, and to only update points and bounds if they have been updated. This makes the solution messier, but should remove the massive slow down. I considered explicitly checking that EDIT: |
⏱️ Performance Benchmark Report: 618d938Performance shiftsFull benchmark resultsGenerated by GHA run |
⏱️ Performance Benchmark Report: 95bc8aaPerformance shiftsFull benchmark resultsGenerated by GHA run |
ESadek-MO
commented
Jul 9, 2025
ESadek-MO
commented
Jul 9, 2025
⏱️ Performance Benchmark Report: f02152ePerformance shiftsFull benchmark resultsGenerated by GHA run |
|
The biggest changes from before the original PR was merged are below.
|
⏱️ Performance Benchmark Report: cc35162Performance shiftsFull benchmark resultsGenerated by GHA run |
|
Benchmarks have retained the improvement, though metadata_equality seems have gotten even faster. I'm not convinced that's meaningful, but might be worth another run locally. EDIT: After running multiple times locally, it's consistantly in the 500s. I'm fine calling it an anomaly, and not rerunning the benchmarks on this PR. |
OK I've re-checked the benchmark results against those from #6465, and I'm totally happy that the remaining shifts are expected and acceptable. |
pp-mo
requested changes
Jul 11, 2025
|
Huge thanks @ESadek-MO |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚀 Pull Request
Investigating #6547.
This has slowed down benchmarks significantly. The cause of this is that every time metadata is updated, so are points and bounds, even if they don't need updating. Updating points and bounds causes significant slowdowns.