-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Shorten some dependency chains in the compiler #145390
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
Conversation
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. These commits modify compiler targets.
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Shorten some dependency chains in the compiler
This comment was marked as outdated.
This comment was marked as outdated.
35d6871
to
54f2e7b
Compare
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Shorten some dependency chains in the compiler
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
(Not sure if we'll see any changes in the bootstrap timings for this, because bootstrap is benchmarked with |
Finished benchmarking commit (0847c11): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 470.224s -> 470.269s (0.01%) |
@Kobzol Fair enough. It's really the CI time that I'm hoping improves here, and that does normal parallel builds. The |
@rust-timer build 0847c11 include=clap_derive |
Why do |
![]() Yeah that's noise. (you can't perf. the same commit twice, btw 😅) |
rustc_attr_parsing needs the id of the item the attribute was applied to for calling |
@bors r- Github says there is a merge conflict. |
`rustc_errors` depends on numerous crates, solely to implement its `IntoDiagArg` trait on types from those crates. Many crates depend on `rustc_errors`, and it's on the critical path. We can't swap things around to make all of those crates depend on `rustc_errors` instead, because `rustc_errors` would end up in dependency cycles. Instead, move `IntoDiagArg` into `rustc_error_messages`, which has far fewer dependencies, and then have most of these crates depend on `rustc_error_messages`. This allows `rustc_errors` to drop dependencies on several crates, including the large `rustc_target`. (This doesn't fully reduce dependency chains yet, as `rustc_errors` still depends on `rustc_hir` which depends on `rustc_target`. That will get fixed in a subsequent commit.)
Some crates depend on `rustc_hir` but only want `HirId` and similar id types. `rustc_hir` is a heavy dependency, since it pulls in `rustc_target`. Split these types out into their own crate `rustc_hir_id`. This allows `rustc_errors` to drop its direct dependency on `rustc_hir`. (`rustc_errors` still depends on `rustc_hir` indirectly through `rustc_lint_defs`; a subsequent commit will fix that.)
`rustc_lint_defs` uses `rustc_hir` solely for the `Namespace` type, which it only needs the static description from. Use the static description directly, to eliminate the dependency on `rustc_hir`. This reduces a long dependency chain: - Many things depend on `rustc_errors` - `rustc_errors` depends on `rustc_lint_defs` - `rustc_lint_defs` depended on `rustc_hir` prior to this commit - `rustc_hir` depends on `rustc_target`
…hir` `rustc_mir_dataflow` only uses `DefId`, which is a re-export from `rustc_span`.
`rustc_traits` only uses `DefId`, which is a re-export from `rustc_span`.
8506be8
to
2ea7e7d
Compare
This PR was rebased onto a different master 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. |
Merge conflict fixed; can be re-queued once CI is green. |
@bors r=lcnr rollup |
…ier, r=lcnr Shorten some dependency chains in the compiler (I recommend reviewing this commit by commit.) One of the long dependency chains in the compiler is: - Many things depend on `rustc_errors`. - `rustc_errors` depended on many things prior to this PR, including `rustc_target`, `rustc_type_ir`, `rustc_hir`, and `rustc_lint_defs`. - `rustc_lint_defs` depended on `rustc_hir` prior to this PR. - `rustc_hir` depends on `rustc_target`. - `rustc_target` is large and takes a while. This PR breaks that chain, through a few steps: - The `IntoDiagArgs` trait, from `rustc_errors`, moves earlier in the dependency chain. This allows `rustc_errors` to stop depending on a pile of crates just to implement `IntoDiagArgs` for their types. - Split `rustc_hir_id` out of `rustc_hir`, so crates that just need `HirId` and similar don't depend on all of `rust_hir` (and thus `rustc_target`). - Make `rustc_lint_defs` stop depending on `rustc_hir`.
Rollup of 19 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145297 (fix(debuginfo): handle false positives in overflow check) - #145390 (Shorten some dependency chains in the compiler) - #145415 (std_detect: RISC-V: implement implication to "C") - #145525 (stdlib: Replace typedef -> type alias in doc comment) - #145590 (Prevent impossible combinations in `ast::ModKind`.) - #145593 (UnsafePinned::raw_get: sync signature with get) - #145621 (Fix some doc typos) - #145627 (Unconditionally-const supertraits are considered not dyn compatible) - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145673 (Add flock support for cygwin) - #145674 (Enable triagebot `[review-changes-since]` feature) - #145678 (Fix typo in docstring) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #145390 - joshtriplett:rustc-diag-value-earlier, r=lcnr Shorten some dependency chains in the compiler (I recommend reviewing this commit by commit.) One of the long dependency chains in the compiler is: - Many things depend on `rustc_errors`. - `rustc_errors` depended on many things prior to this PR, including `rustc_target`, `rustc_type_ir`, `rustc_hir`, and `rustc_lint_defs`. - `rustc_lint_defs` depended on `rustc_hir` prior to this PR. - `rustc_hir` depends on `rustc_target`. - `rustc_target` is large and takes a while. This PR breaks that chain, through a few steps: - The `IntoDiagArgs` trait, from `rustc_errors`, moves earlier in the dependency chain. This allows `rustc_errors` to stop depending on a pile of crates just to implement `IntoDiagArgs` for their types. - Split `rustc_hir_id` out of `rustc_hir`, so crates that just need `HirId` and similar don't depend on all of `rust_hir` (and thus `rustc_target`). - Make `rustc_lint_defs` stop depending on `rustc_hir`.
(I recommend reviewing this commit by commit.)
One of the long dependency chains in the compiler is:
rustc_errors
.rustc_errors
depended on many things prior to this PR, includingrustc_target
,rustc_type_ir
,rustc_hir
, andrustc_lint_defs
.rustc_lint_defs
depended onrustc_hir
prior to this PR.rustc_hir
depends onrustc_target
.rustc_target
is large and takes a while.This PR breaks that chain, through a few steps:
IntoDiagArgs
trait, fromrustc_errors
, moves earlier in the dependency chain. This allowsrustc_errors
to stop depending on a pile of crates just to implementIntoDiagArgs
for their types.rustc_hir_id
out ofrustc_hir
, so crates that just needHirId
and similar don't depend on all ofrust_hir
(and thusrustc_target
).rustc_lint_defs
stop depending onrustc_hir
.