Add support for artifact size profiling by rylev · Pull Request #87404 · rust-la...
source link: https://github.com/rust-lang/rust/pull/87404
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
None yet
Successfully merging this pull request may close these issues.
None yet
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
This comment has been hidden.
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 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.
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.
rustc_incremental already depends on rustc_query_impl (directly or through rustc_middle). You can export a const to be accessed by rustc_incremental.
I think that would actually introduce a circular dependency, right?
rustc_query_system -> rustc_incremental -> rustc_middle
^ |
| |
+--------------------------------------+
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.
Just one question. r=me when you're ready to merge
compiler/rustc_data_structures/src/profiling.rs
Show resolved
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.
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.
Commit 757f76e has been approved by wesleywiser
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK