break rustc_expand-rustc_middle dependency

[mejrs]

View all comments

Back in #145354 (cc @Kobzol), support for caching derive macros was added. With this, rustc_expand was made to depend on rustc_middle.

This PR breaks that dependency so that rustc_expand and rustc_builtin_macros can compile in parallel with rustc_middle.

The timing graph goes from this:

to this:

Note that the interval where we're exclusively compiling rustc_middle has become much shorter.
Full graphs here:
cargo-timing-main.html
cargo-timing-expand2.html

It's somewhat hard to benchmark reliably but a full bootstrap is about 5-10 seconds faster overall, on a machine with an Amd Ryzen 5900x 12-core processor.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in src/tools/cargo

cc @weihanglo

[rustbot]

r? @JonathanBrouwer

rustbot has assigned @JonathanBrouwer.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 75 candidates
• Random selection from 15 candidates

[mejrs]

Shouldn't show up in single job benchmarks, but still:

I'll also do another benchmark later where the derive expander is always called through the atomic ref (with enable called depending on whether derive macros are cached or not)

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: e3517d8 (e3517d88b373f392501ed15f89f9f57cd5370ddf)
Base parent: e702eca (e702ecae8a3e3756e8a499f8592be1e3efac0e8f)

[rust-timer]

Finished benchmarking commit (e3517d8): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking 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 count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary 2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.4%, 2.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.1%]	72
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	114
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.1%]	72

Bootstrap: 457.751s -> 466.084s (1.82%)
Artifact size: 398.90 MiB -> 398.92 MiB (0.01%)

[mejrs]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 8fa19ca (8fa19caf36a75113f80ef0db3ec480a54b3d7329)
Base parent: cda599c (cda599c55f56d6cc234352f267102c293e55f3d0)

[rust-timer]

Finished benchmarking commit (8fa19ca): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking 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 count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary -1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-1.7%, -1.7%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 2.0%, secondary 0.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.0%	[2.0%, 2.0%]	1
Regressions ❌ (secondary)	3.4%	[3.4%, 3.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-1.8%, -1.8%]	1
All ❌✅ (primary)	2.0%	[2.0%, 2.0%]	1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 455.4s -> 455.94s (0.12%)
Artifact size: 398.90 MiB -> 398.97 MiB (0.02%)

[JonathanBrouwer]

A best case scenario

./x check compiler/rustc_expand

Goes from 15.35s -> 11.44s for me, very cool.
With a 9950X processor.

[JonathanBrouwer]

Making sure removing the last commit did not affect perf
@bors try @rust-timer queue

[JonathanBrouwer]

kobzol is not on the review rotation at the moment. They may take a while to respond.

Maybe instead
r? petrochenkov

[rustbot]

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.

[mejrs]

@rustbot ready

[petrochenkov]

Any specific reason not to use a closure here? They'll coerce into a fn pointer too.

View changes since the review

[petrochenkov]

r=me with a closure #161263 (comment), unless there's a reason not to use it.
@rustbot author

[mejrs]

Changed to use a closure.

@bors r=petrochenkov

[rust-bors]

📌 Commit de40569 has been approved by petrochenkov

It is now in the queue for this repository.