typeck in parallel #113519

SparrowLii · 2023-07-10T03:01:48Z

#108118 caused typeck to be transferred to the serial part (check_unused), which made the performance of parallel rustc significantly reduced.

This pr re-parallelize this part, which increases the average performance improvement of parallel rustc in full and incr-full scenarios from 14.4% to 23.2%.

r? @cjgillot
cc @oli-obk @Zoxc

SparrowLii · 2023-07-10T03:08:59Z

Because AonoConsts also belong to body_owners, but they should be fed in their parents' typeck, so we skip them.
According to @oli-obk, we would creat the DefIds for AonoConsts in their parents' typeck in the future. We could get rid of this patch and directly parallelize the iterators in other processes then.

cjgillot

This same function iterates over modules serially. Should those call some par_for_each_module method?
Probably to be decided by looking at profiles.

compiler/rustc_hir_analysis/src/lib.rs

cjgillot · 2023-07-10T10:58:12Z

compiler/rustc_hir_analysis/src/lib.rs

+    // FIXME: Parallelize other iterators(e.g. `body_owners` in `check_unused`) instead
+    // when we implement creating `DefId`s for anon constants during their parents' typeck.
+    // Since doing so will produce query cycle errors at current.
+    if cfg!(parallel_compiler) {


I'm not sure we should have different logic for serial vs parallel compiler.

Actually it doesn't change the logic, because check_unused will call all typeck and fetch the used_trait_impl information in the results. I moved this part into check_unused to make it look more reasonable.

This code really belongs in check_crate, and not in check_unused. My suggestion was just to remove the if cfg!(parallel_compiler), and run this unconditionally.

This reverts part of #108118. I agree this is reasonable.

I removed cfg!(parallel_compiler) and currented some ui tests, since the order of errors during typeck is changed

cjgillot · 2023-07-10T10:59:14Z

compiler/rustc_hir_analysis/src/lib.rs

@@ -494,6 +494,18 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
        tcx.hir().for_each_module(|module| tcx.ensure().check_mod_item_types(module))
    });

+    // FIXME: Parallelize other iterators(e.g. `body_owners` in `check_unused`) instead


I don't understand this sentence. Should the FIXME be in `check_unused?

Yea, I moved this part into check_unused to make it look more reasonable.

I just add a FIXME above check_unused now

SparrowLii · 2023-07-10T11:24:59Z

Thanks for reviewing! If you mean item_types_checking, It has little effect on the profile so I didn't modify it.

compiler/rustc_hir_analysis/src/lib.rs

SparrowLii · 2023-07-11T09:47:34Z

I don't understand why the clippy test failed suddenly when I fixed the comment

oli-obk · 2023-07-11T10:29:31Z

That's a spurious CI bug that I am looking into.

cjgillot · 2023-07-11T16:44:31Z

@bors try @rust-timer queue

bors · 2023-07-11T16:44:40Z

⌛ Trying commit 50896c1 with merge 96389d030446cf91feda468c7b2c7a10ff67e35b...

bors · 2023-07-11T18:01:06Z

☀️ Try build successful - checks-actions
Build commit: 96389d030446cf91feda468c7b2c7a10ff67e35b (96389d030446cf91feda468c7b2c7a10ff67e35b)

rust-timer · 2023-07-11T22:06:27Z

Finished benchmarking commit (96389d030446cf91feda468c7b2c7a10ff67e35b): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.5%	[3.5%, 3.5%]	1
Regressions ❌ (secondary)	3.8%	[3.1%, 4.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-3.0%, -3.0%]	1
All ❌✅ (primary)	3.5%	[3.5%, 3.5%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 656.645s -> 656.709s (0.01%)

cjgillot · 2023-07-13T09:47:21Z

@bors r+

bors · 2023-07-13T09:47:22Z

📌 Commit 50896c1 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-07-14T03:47:05Z

⌛ Testing commit 50896c1 with merge 7d60819...

bors · 2023-07-14T05:30:46Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 7d60819 to master...

rust-timer · 2023-07-14T08:29:09Z

Finished benchmarking commit (7d60819): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.4%	[3.0%, 4.0%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 659.456s -> 656.192s (-0.49%)

rustbot assigned cjgillot Jul 10, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2023

SparrowLii mentioned this pull request Jul 10, 2023

Tracking Issue for Parallel Rustc Front-end #113349

Open

28 tasks

cjgillot reviewed Jul 10, 2023

View reviewed changes

SparrowLii force-pushed the parallel_typeck branch from b746d7a to a88a4fb Compare July 11, 2023 02:04

This comment has been minimized.

Sign in to view

SparrowLii force-pushed the parallel_typeck branch from a88a4fb to 47a2662 Compare July 11, 2023 02:52

oli-obk reviewed Jul 11, 2023

View reviewed changes

compiler/rustc_hir_analysis/src/lib.rs Outdated Show resolved Hide resolved

SparrowLii force-pushed the parallel_typeck branch from 47a2662 to 983b4ae Compare July 11, 2023 09:21

oli-obk approved these changes Jul 11, 2023

View reviewed changes

SparrowLii force-pushed the parallel_typeck branch from 983b4ae to 692e5d9 Compare July 11, 2023 09:25

This comment has been minimized.

Sign in to view

typeck in parallel

50896c1

SparrowLii force-pushed the parallel_typeck branch from 692e5d9 to 50896c1 Compare July 11, 2023 11:39

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 11, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 11, 2023

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 Jul 13, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 14, 2023

bors merged commit 7d60819 into rust-lang:master Jul 14, 2023

rustbot added this to the 1.73.0 milestone Jul 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

typeck in parallel #113519

typeck in parallel #113519

SparrowLii commented Jul 10, 2023

SparrowLii commented Jul 10, 2023

cjgillot left a comment

cjgillot Jul 10, 2023

SparrowLii Jul 10, 2023

cjgillot Jul 10, 2023

SparrowLii Jul 11, 2023

SparrowLii Jul 11, 2023 •

edited

Loading

cjgillot Jul 10, 2023

SparrowLii Jul 10, 2023 •

edited

Loading

SparrowLii Jul 11, 2023

SparrowLii commented Jul 10, 2023 •

edited

Loading

This comment has been minimized.

SparrowLii commented Jul 11, 2023 •

edited

Loading

This comment has been minimized.

oli-obk commented Jul 11, 2023

cjgillot commented Jul 11, 2023

This comment has been minimized.

bors commented Jul 11, 2023

bors commented Jul 11, 2023

This comment has been minimized.

rust-timer commented Jul 11, 2023

cjgillot commented Jul 13, 2023

bors commented Jul 13, 2023

bors commented Jul 14, 2023

bors commented Jul 14, 2023

rust-timer commented Jul 14, 2023

typeck in parallel #113519

typeck in parallel #113519

Conversation

SparrowLii commented Jul 10, 2023

SparrowLii commented Jul 10, 2023

cjgillot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparrowLii Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparrowLii Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparrowLii commented Jul 10, 2023 • edited Loading

This comment has been minimized.

SparrowLii commented Jul 11, 2023 • edited Loading

This comment has been minimized.

oli-obk commented Jul 11, 2023

cjgillot commented Jul 11, 2023

This comment has been minimized.

bors commented Jul 11, 2023

bors commented Jul 11, 2023

This comment has been minimized.

rust-timer commented Jul 11, 2023

Overall result: no relevant changes - no action needed

cjgillot commented Jul 13, 2023

bors commented Jul 13, 2023

bors commented Jul 14, 2023

bors commented Jul 14, 2023

rust-timer commented Jul 14, 2023

Overall result: no relevant changes - no action needed

SparrowLii Jul 11, 2023 •

edited

Loading

SparrowLii Jul 10, 2023 •

edited

Loading

SparrowLii commented Jul 10, 2023 •

edited

Loading

SparrowLii commented Jul 11, 2023 •

edited

Loading