Move attributes out of rustc_hir - #160336
Conversation
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_hir/src/attrs |
|
@rustbot try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0b1c8e0 to
cafdff7
Compare
There was a problem hiding this comment.
should this crate have a readme?
There was a problem hiding this comment.
Maybe but not in this PR, I want to keep this PR as concise as I can.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Move attributes out of rustc_hir
|
Oops |
|
By the way I did a perf run on the other PR as well, see https://perf.rust-lang.org/compare.html?start=29e68fe2295f8fc2feb52b8cb0b61a055842fdcf&end=5bfd1598fde1f19d34aa3199461963dcce69bb0a&stat=instructions%3Au&tab=bootstrap But the rustc_symbol_mangling result is weird, let's see if that happens again. |
|
@mejrs What did you use to graph the dependencies? |
Also take a look at the |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (8ff9bde): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary -6.2%, secondary 0.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.5%, secondary -0.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 489.711s -> 487.73s (-0.40%) |
|
@mejrs wrote:
Thanks!
I'm familiar; I added the support for doing (I just sent a PR to rustc-dev-guide to document that.) |
This comment has been minimized.
This comment has been minimized.
|
(will take a look at this on friday at the latest, I hope rebasing until then is not too annoying) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
☔ The latest upstream changes (presumably #160517) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
Remove rustc_middle dependency on rustc_hir_pretty There is a `impl PpAnn for TyCtxt` that is unneeded. None of the big crates (middle, trait_selection) actually do any hir pretty printing so it can be removed and can either be implemented for local structs elsewhere or done by casting to `&dyn PpAnn` instead. This probably doesn't have any perf effects (its not bottlenecking in timings) but I expect it to compose with rust-lang#160336 and its followup works quite well. Old graph: <img width="1999" height="2171" alt="graph" src="https://github.com/user-attachments/assets/756e6a50-a413-4a8d-a0f3-7bbf4de07933" /> New graph: <img width="1853" height="2171" alt="graph_hir_pretty" src="https://github.com/user-attachments/assets/f9607e5a-d00b-4a86-8f6a-cefb45b75ad7" />
Remove rustc_middle dependency on rustc_hir_pretty There is a `impl PpAnn for TyCtxt` that is unneeded. None of the big crates (middle, trait_selection) actually do any hir pretty printing so it can be removed and can either be implemented for local structs elsewhere or done by casting to `&dyn PpAnn` instead. This probably doesn't have any perf effects (its not bottlenecking in timings) but I expect it to compose with rust-lang#160336 and its followup works quite well. Old graph: <img width="1999" height="2171" alt="graph" src="https://github.com/user-attachments/assets/756e6a50-a413-4a8d-a0f3-7bbf4de07933" /> New graph: <img width="1853" height="2171" alt="graph_hir_pretty" src="https://github.com/user-attachments/assets/f9607e5a-d00b-4a86-8f6a-cefb45b75ad7" />
Remove rustc_middle dependency on rustc_hir_pretty There is a `impl PpAnn for TyCtxt` that is unneeded. None of the big crates (middle, trait_selection) actually do any hir pretty printing so it can be removed and can either be implemented for local structs elsewhere or done by casting to `&dyn PpAnn` instead. This probably doesn't have any perf effects (its not bottlenecking in timings) but I expect it to compose with rust-lang#160336 and its followup works quite well. Old graph: <img width="1999" height="2171" alt="graph" src="https://github.com/user-attachments/assets/756e6a50-a413-4a8d-a0f3-7bbf4de07933" /> New graph: <img width="1853" height="2171" alt="graph_hir_pretty" src="https://github.com/user-attachments/assets/f9607e5a-d00b-4a86-8f6a-cefb45b75ad7" />
Remove rustc_middle dependency on rustc_hir_pretty There is a `impl PpAnn for TyCtxt` that is unneeded. None of the big crates (middle, trait_selection) actually do any hir pretty printing so it can be removed and can either be implemented for local structs elsewhere or done by casting to `&dyn PpAnn` instead. This probably doesn't have any perf effects (its not bottlenecking in timings) but I expect it to compose with rust-lang#160336 and its followup works quite well. Old graph: <img width="1999" height="2171" alt="graph" src="https://github.com/user-attachments/assets/756e6a50-a413-4a8d-a0f3-7bbf4de07933" /> New graph: <img width="1853" height="2171" alt="graph_hir_pretty" src="https://github.com/user-attachments/assets/f9607e5a-d00b-4a86-8f6a-cefb45b75ad7" />
Remove rustc_middle dependency on rustc_hir_pretty There is a `impl PpAnn for TyCtxt` that is unneeded. None of the big crates (middle, trait_selection) actually do any hir pretty printing so it can be removed and can either be implemented for local structs elsewhere or done by casting to `&dyn PpAnn` instead. This probably doesn't have any perf effects (its not bottlenecking in timings) but I expect it to compose with rust-lang#160336 and its followup works quite well. Old graph: <img width="1999" height="2171" alt="graph" src="https://github.com/user-attachments/assets/756e6a50-a413-4a8d-a0f3-7bbf4de07933" /> New graph: <img width="1853" height="2171" alt="graph_hir_pretty" src="https://github.com/user-attachments/assets/f9607e5a-d00b-4a86-8f6a-cefb45b75ad7" />
View all comments
Previous graph:
Graph with this pr:
So while it doesn't really look like much (might be nicer for pipeling or if you're editing rustc_hir), it unblocks followup work to split up this part of the crate graph further, like:
SanitizerSetto a (new?) crate belowrustc_targetso thatrustc_targetandrustc_attr_irno longer depend on each other and can compile in parallelrustc_sessionwill only depend onrustc_hirforrustc_hir::definitionsin itscstoremodule, there may be something there but I haven't looked in depth.There's some work left wrt cleaning up imports everywhere, but that's for followups.
r? @JonathanBrouwer