-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Track PGO profiles in depinfo #100801
Track PGO profiles in depinfo #100801
Conversation
This comment has been minimized.
This comment has been minimized.
0a9e256
to
aec34d6
Compare
Looks reasonable, but I am not a reviewer on the rust repo. |
📌 Commit aec34d6ed5d3ef87b34d4e6323848160f5d97298 has been approved by It is now in the queue for this repository. |
⌛ Testing commit aec34d6ed5d3ef87b34d4e6323848160f5d97298 with merge 71b88e490749a284c99f46df5bad873b27a413c3... |
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
f8e3336
to
925644e
Compare
Fixed the MSVC CI failure. @rustbot ready |
@michaelwoerister Probably the PR will have to be reapproved. |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ff479b1): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Footnotes |
PGO is not used when compiling the perf. suite benchmarks and keccak has been bimodal lately, so this is just noise. @rustbot label: +perf-regression-triaged |
This PR makes sure that PGO profiles (
-Cprofile-use
and-Cprofile-sample-use
) are tracked in depinfo, so that when they change, the compilation session will be invalidated.This approach was discussed on Zulip.
I tried it locally and it seems that the code is recompiled just with this change, and #100413 is not even needed. But it's possible that not everything required is recompiled, so we will probably want to land both changes.
Another approach to implement this could be to store the PGO profiles in
sess.parse_sess.file_depinfo
when the session is being created, but then the paths would have to be converted to a string and then to a symbol, which seemed unnecessarily complicated.CC @michaelwoerister
r? @Eh2406