split up rustc_session

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.

[mejrs]

Followup on #160336 cc @JonathanBrouwer (feel free to review if you want)

The first commit splits out the cstore module from rustc_session into its own crate.

rustc_session actually never used it anywhere internally, and there are also several crates that only depend on rustc_session for it. So it is a natural candidate to split off.

The second commit is similar; rustc_hir::definitions is moved to rustc_hir_id; it is used nowhere in rustc_hir, except to implement an inherent method:

implDefKind{pubfndef_path_data(self,name:Option<Symbol>) -> DefPathData{}}

which is used (once) in rustc_middle, so the items in rustc_hir::definitions and this conversion function could be in many places. rustc_hir_id is the earliest and the easiest, so that's what I went with.

Together these allow rustc_crate_store, rustc_hir and rustc_session to compile in parallel.
Current graph:

Graph after this pr:

[rustbot]

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

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.

[rustbot]

r? @mu001999

rustbot has assigned @mu001999.
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 18 candidates

[JonathanBrouwer]

@bors try @rust-timer queue
Just sanity checking the bootstrap time results, not expecting a difference though since as you mentioned bootstrap times are single threaded

[JonathanBrouwer]

Happy to review this, should have time
r? me

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3dcaed8 (3dcaed8817c1af47e6557c39d4b7848f7db2af6c)
Base parent: a04c7a0 (a04c7a037d59b6dedc9f921e84e391f13a257aa4)

[rust-timer]

Finished benchmarking commit (3dcaed8): 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 0.5%)

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.3%	[0.6%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-3.0%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.3%)

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.9%	[0.4%, 3.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 463.191s -> 456.228s (-1.50%)
Artifact size: 396.43 MiB -> 396.39 MiB (-0.01%)

[JonathanBrouwer]

Code changes look good, and intuitively this also looks like an improvement but it's always good to test, so:

I did some perf testing locally on this PR, it would be nice if rust-timer had multi-threaded bootstrap benchmarks but that's sadly not the world we live in. Running ./x check compiler/rustc_middle from a clean build directory takes 14.7s-14.8s before this PR, and 14.2s-14.3s after this PR on my quite beefy PC. Average of 10 runs. That's quite a nice improvement!

Script that I ran was (not proud of this one)

foriin$(seq 1 10);do rm -r build/x86_64-unknown-linux-gnu/stage1-rustc/;time ./x -q check compiler/rustc_middle;done

@bors r+ rollup

[rust-bors]

📌 Commit 37c704b has been approved by JonathanBrouwer

It is now in the queue for this repository.