Skip to content
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

[self-profiling] Include the estimated size of each cgu in the profile #78702

Merged
merged 1 commit into from
Nov 17, 2020

Conversation

wesleywiser
Copy link
Member

This is helpful when looking for CGUs where the size estimate isn't a
good indicator of compilation time.

I verified that moving the profiling timer call doesn't affect the
results.

Results:

Screen Shot 2020-11-03 at 7 25 04 AM

measureme doesn't have support for custom arg names yet so arg0 is the CGU name and arg1 is the estimated size.

@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 3, 2020
This is helpful when looking for CGUs where the size estimate isn't a
good indicator of compilation time.

I verified that moving the profiling timer call doesn't affect the
results.
@jyn514 jyn514 added I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 4, 2020
@wesleywiser wesleywiser added A-self-profile Area: Self-profiling feature of the compiler and removed I-heavy Issue: Problems and improvements with respect to binary size of generated code. labels Nov 5, 2020
@lcnr
Copy link
Contributor

lcnr commented Nov 9, 2020

r? @Mark-Simulacrum

@rust-highfive rust-highfive assigned Mark-Simulacrum and unassigned lcnr Nov 9, 2020
let _prof_timer = tcx.prof.generic_activity_with_args(
"codegen_module",
&[cgu_name.to_string(), cgu.size_estimate().to_string()],
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this now excludes the codegen_unit query -- is that intentional? I guess the query was always pre-executed which is why results are not affected?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The codegen_unit query was already invoked just before running the task of module_codegen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but previously that was included in the timer here I think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the query is certainly run before this point so the only difference is the fast query lookup and the fact the query is run inside this activity instead of right before it. I don't think moving the query out of the activity is significant so that seemed fine to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, makes sense. Thanks!

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 14, 2020
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 17, 2020

📌 Commit efe703a has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 17, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2020
Rollup of 8 pull requests

Successful merges:

 - rust-lang#74293 (Rustdoc test compiler output color)
 - rust-lang#78702 ([self-profiling] Include the estimated size of each cgu in the profile)
 - rust-lang#79069 (Get rid of `highlight::Class::None`)
 - rust-lang#79072 (Fix exhaustiveness in case a byte string literal is used at slice type)
 - rust-lang#79120 (update rustfmt to v1.4.27)
 - rust-lang#79125 (Get rid of clean::{Method, TyMethod})
 - rust-lang#79126 (Remove duplicate `Trait::auto` field)
 - rust-lang#79130 (extend macro braces test)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fa45fce into rust-lang:master Nov 17, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-self-profile Area: Self-profiling feature of the compiler S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants