Increase parallelism in various locations #115003

Zoxc · 2023-08-19T17:32:51Z

This is a rebase of #67870 which consists of various additions of parallelism in the compiler.

Performance improvement for the parallel compiler with 6 threads:

Benchmark	Before	After
Benchmark	Time	Time	%
🟣 clap:check	0.7382s	0.6639s	💚 -10.07%
🟣 hyper:check	0.1873s	0.1609s	💚 -14.10%
🟣 regex:check	0.4478s	0.4173s	💚 -6.81%
🟣 syn:check	0.7475s	0.6857s	💚 -8.26%
🟣 syntex_syntax:check	2.3616s	2.1652s	💚 -8.32%
Total	4.4823s	4.0929s	💚 -8.69%
Summary	1.0000s	0.9049s	💚 -9.51%

cc @SparrowLii

rustbot · 2023-08-19T17:32:56Z

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

compiler/rustc_hir_analysis/src/lib.rs

cjgillot · 2023-08-20T09:08:08Z

compiler/rustc_interface/src/passes.rs

@@ -757,14 +757,28 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
                CStore::from_tcx(tcx).report_unused_deps(tcx);
            },
            {
+                // Prefetch this as it is used later by the loop below
+                // to prevent multiple threads from blocking on it.
+                let _ = tcx.get_lang_items(());


Can this be fixed in query infra, yielding control to another job if blocked on another query?

Somewhat by using a thread pool with fibers. That's not a short term fix and it's not free, so we might want to keep these anyway.

compiler/rustc_hir_analysis/src/collect.rs

cjgillot · 2023-08-20T09:11:49Z

compiler/rustc_hir_analysis/src/lib.rs

+                // FIXME: Remove this when we implement creating `DefId`s
+                // for anon constants during their parents' typeck.
+                // Typeck all body owners in parallel will produce queries
+                // cycle errors because it may typeck on anon constants directly.


Is the cycle issue fixed?

This PR just moves this code. It shouldn't have any effect on any cycle issue.

cjgillot · 2023-08-20T09:18:57Z

compiler/rustc_interface/src/passes.rs

@@ -805,7 +805,8 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
    sess.time("misc_checking_2", || {
        parallel!(
            {
-                tcx.ensure().effective_visibilities(());
+                // Prefetch this as it is used later by lint checking and privacy checking.
+                let _ = tcx.effective_visibilities(());


Why isn't the ensure enough? effective_visibilities is eval_always.

I've changed all the prefetching to use ensure_with_value. It's nicer than having to rely on queries being eval_always which might possibly change.

cjgillot · 2023-08-20T09:19:41Z

tests/ui/const-generics/issues/issue-83765.rs

        self.reference.bget(index).map(&self.closure)
+        //~^ ERROR unconstrained generic constant
+        //~| ERROR mismatched types


Where do the additional diags come from?

Compilation no long immediately stops on fatal errors and instead wait for all parallel executions to be complete. The serial compiler simulates this behavior to ensure test outputs match.

compiler/rustc_interface/src/passes.rs

cjgillot · 2023-08-20T09:20:58Z

@bors try @rust-timer queue

bors · 2023-08-20T09:21:07Z

⌛ Trying commit f9d12e2a6022cbbed60f458a78502791ec77ccef with merge 5f30bb0cbc3842d6520bacf4d7dea860e138c4d9...

bors · 2023-08-20T10:34:21Z

☀️ Try build successful - checks-actions
Build commit: 5f30bb0cbc3842d6520bacf4d7dea860e138c4d9 (5f30bb0cbc3842d6520bacf4d7dea860e138c4d9)

rust-timer · 2023-08-20T12:43:56Z

Finished benchmarking commit (5f30bb0cbc3842d6520bacf4d7dea860e138c4d9): comparison URL.

Overall result: ❌ regressions - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	4.2%	[0.4%, 87.3%]	126
Regressions ❌ (secondary)	11.6%	[0.2%, 77.0%]	166
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.2%	[0.4%, 87.3%]	126

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.8%	[0.6%, 19.5%]	117
Regressions ❌ (secondary)	8.1%	[1.3%, 23.5%]	196
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	3.8%	[0.6%, 19.5%]	117

Cycles

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)	6.5%	[1.2%, 68.7%]	65
Regressions ❌ (secondary)	14.6%	[1.9%, 61.4%]	115
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	6.5%	[1.2%, 68.7%]	65

Binary size

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.0%	[0.0%, 0.0%]	2
Regressions ❌ (secondary)	0.1%	[0.0%, 0.3%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	2

Bootstrap: 636.001s -> 635.653s (-0.05%)
Artifact size: 346.92 MiB -> 347.11 MiB (0.06%)

Zoxc · 2023-08-20T14:38:46Z

I avoided prefetching of visible_parent_map as that wasn't always executed. This could use another perf run now.

lqd · 2023-08-20T16:39:04Z

@bors try @rust-timer queue

bors · 2023-08-20T16:39:13Z

⌛ Trying commit d125a5d8c1dc23342aa5641675df34d33a76958f with merge 78a3877375fe23c25e4c0ea3ff01b84d806cd35d...

bors · 2023-08-20T17:50:48Z

☀️ Try build successful - checks-actions
Build commit: 78a3877375fe23c25e4c0ea3ff01b84d806cd35d (78a3877375fe23c25e4c0ea3ff01b84d806cd35d)

rust-timer · 2023-08-20T19:13:06Z

Finished benchmarking commit (78a3877375fe23c25e4c0ea3ff01b84d806cd35d): comparison URL.

Overall result: ❌ regressions - 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 is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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)	1.1%	[1.1%, 1.1%]	1
Regressions ❌ (secondary)	3.2%	[2.5%, 3.9%]	2
Improvements ✅ (primary)	-3.7%	[-3.7%, -3.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.3%	[-3.7%, 1.1%]	2

Cycles

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)	2.9%	[2.9%, 2.9%]	2
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.1%, -1.1%]	1

Binary size

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

Bootstrap: 636.195s -> 635.468s (-0.11%)
Artifact size: 346.98 MiB -> 347.12 MiB (0.04%)

bors · 2023-08-27T18:45:37Z

☔ The latest upstream changes (presumably #115226) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2023-08-30T22:13:11Z

☔ The latest upstream changes (presumably #115144) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2023-09-06T13:51:30Z

☔ The latest upstream changes (presumably #115401) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2023-10-25T02:33:34Z

☔ The latest upstream changes (presumably #116482) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-01-19T11:54:41Z

☔ The latest upstream changes (presumably #120112) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-03-07T13:10:16Z

☔ The latest upstream changes (presumably #121154) made this pull request unmergeable. Please resolve the merge conflicts.

oskgo · 2024-08-09T15:11:57Z

@Zoxc Any updates on this? Thanks

Zoxc · 2024-08-09T15:15:52Z

You can probably consider this blocked on rust-lang/rustc-rayon#12.

rustbot assigned compiler-errors Aug 19, 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 Aug 19, 2023

cjgillot self-assigned this Aug 20, 2023

cjgillot reviewed Aug 20, 2023

View reviewed changes