Add support for artifact size profiling #87404
Conversation
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
inquisitivecrystal
added
the
T-compiler
|
☔ The latest upstream changes (presumably #83214) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@rylev the measureme PR was merged it looks like :) do you know when you'll get a chance to follow up? |
jyn514
added
S-waiting-on-author
|
@jyn514 I will be getting to this soon. We first wanted to make sure our changes to measureme and the related tooling don’t break perf.rlo. This should be easy to test, I just haven’t found the time yet. |
rylev
force-pushed
the
artifact-size-profiling
branch
from
f32353e
to
757f76e
Compare
|
An update: we're waiting on rust-lang/rustc-perf#1063 to be merged and to see that no issues arise from it. Once that happens, we should be good to go here. One concern is that miri and rustc-ap-rustc_data_structures still use 0.9.2. I believe rustc-ap-rustc_data_structures auto-updates but I'm not sure. |
|
This is good to go. I'd like to see this land, and then we can start working on rustc-perf to expose some of these stats in the perf.rlo UI. |
| encoder.flush() | ||
| let result = encoder.flush(); | ||
| // FIXME(rylev): we hardcode the dep graph file name so we don't need a dependency on | ||
| // rustc_incremental just for that. |
There was a problem hiding this comment.
rustc_incremental already depends on rustc_query_impl (directly or through rustc_middle). You can export a const to be accessed by rustc_incremental.
There was a problem hiding this comment.
I think that would actually introduce a circular dependency, right?
rustc_query_system -> rustc_incremental -> rustc_middle
^ |
| |
+--------------------------------------+
There was a problem hiding this comment.
I believe the failure mode here if the dep graph file name changes is just that we have the wrong file name in the self-profile data which doesn't seem important enough to add the dependency, so this seems fine.
| encoder.flush() | ||
| let result = encoder.flush(); | ||
| // FIXME(rylev): we hardcode the dep graph file name so we don't need a dependency on | ||
| // rustc_incremental just for that. |
There was a problem hiding this comment.
I believe the failure mode here if the dep graph file name changes is just that we have the wrong file name in the self-profile data which doesn't seem important enough to add the dependency, so this seems fine.
|
@bors r+ |
|
📌 Commit 757f76e has been approved by |
bors
added
S-waiting-on-bors
Rollup of 10 pull requests Successful merges: - rust-lang#86479 (Automatic exponential formatting in Debug) - rust-lang#87404 (Add support for artifact size profiling) - rust-lang#87769 (Alloc features cleanup) - rust-lang#88789 (remove unnecessary bound on Zip specialization impl) - rust-lang#88860 (Deduplicate panic_fmt) - rust-lang#90009 (Make more `From` impls `const` (libcore)) - rust-lang#90018 (Fix rustdoc UI for very long type names) - rust-lang#90025 (Revert rust-lang#86011 to fix an incorrect bound check) - rust-lang#90036 (Remove border-bottom from most docblocks.) - rust-lang#90060 (Update RELEASES.md) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This adds support for profiling artifact file sizes (incremental compilation artifacts and query cache to begin with).
Eventually we want to track this in perf.rlo so we can ensure that file sizes do not change dramatically on each pull request.
This relies on support in measureme: rust-lang/measureme#169. Once that lands we can update this PR to not point to a git dependency.
This was worked on together with @michaelwoerister.
r? @wesleywiser