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

Improve HIR stats #101142

Merged
merged 8 commits into from
Sep 5, 2022
Merged

Improve HIR stats #101142

merged 8 commits into from
Sep 5, 2022

Conversation

nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Aug 29, 2022

#100398 improve the AST stats collection done by -Zhir-stats. This PR does the same for HIR stats collection.

r? @davidtwco

This is based on `-Zprint-type-sizes` which does the same thing. It
makes the output provenance clearer, and helps with post-processing.
E.g. if you have `-Zhir-stats` output from numerous compiler invocations
you can now easily extract the pre-expansion stats separately from the
post-expansion stats.
For consistency, and because it makes HIR measurement simpler and more
accurate.
For consistency, and because it makes HIR measurement simpler and more
accurate.
For consistency, and because it makes HIR measurement simpler and more
accurate.
This comment on the HIR `visit_path_segment` is supposed be on the AST
`visit_path_segment`.
Because `hir_id` is the standard name for methods that return a `HirId`
from a HIR node.
Adds and removes some `visit_*` methods accordingly, improving
coverage, and avoiding some double counting. Brings it in line with the
AST stats collector.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 29, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2022
@nnethercote
Copy link
Contributor Author

Best reviewed one commit at a time.

@nnethercote
Copy link
Contributor Author

I learned that @michaelwoerister is on leave. Let's try another reviewer.

r? @davidtwco

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

This looks good to me, one nit, feel free to resolve it or not, r=me.

@@ -416,7 +416,7 @@ pub fn configure_and_expand(
}

if sess.opts.unstable_opts.hir_stats {
hir_stats::print_ast_stats(&krate, "POST EXPANSION AST STATS");
hir_stats::print_ast_stats(&krate, "POST EXPANSION AST STATS", "ast-stats-2");
Copy link
Member

@davidtwco davidtwco Sep 2, 2022

Choose a reason for hiding this comment

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

nit: ast-stats-2 could probably be ast-post-stats or something a little more descriptive? Likewise with ast-stats-1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave it as is. Anything short of ast-stats-{pre,post}-expansion is going to be non-obvious to a first-time user, but that's a bit on the long side. And the heading ensures that a first-time user will be able to quickly work out the meaning of the 1 and 2.

@nnethercote
Copy link
Contributor Author

@bors r=davidtwco

@bors
Copy link
Contributor

bors commented Sep 5, 2022

📌 Commit f26fdce has been approved by davidtwco

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#101142 (Improve HIR stats)
 - rust-lang#101367 (Suggest `{Option,Result}::{copied,clone}()` to satisfy type mismatch)
 - rust-lang#101391 (more clippy::perf fixes)
 - rust-lang#101409 (Don't fire `rust_2021_incompatible_closure_captures` in `edition = 2021` crates)
 - rust-lang#101420 (Fix `hir::Local` doc to match with the variable name used: `init`)
 - rust-lang#101429 (Don't suggest reborrow if usage is inside a closure)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5d55009 into rust-lang:master Sep 5, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 5, 2022
@nnethercote nnethercote deleted the improve-hir-stats branch September 5, 2022 23:00
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 9, 2022
…avidtwco

Improve HIR stats

rust-lang#100398 improve the AST stats collection done by `-Zhir-stats`. This PR does the same for HIR stats collection.

r? `@davidtwco`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants