catch unwind in parallel mode during wfcheck #97307

SparrowLii · 2022-05-23T07:53:45Z

Update #75760
When performing wfcheck, from the test results, the parallel mode will stop all checks when an item's check failed, (e.g. the first ui test failure raised from here)while the serial mode will output each item's check result via catch_unwind. This leads to inconsistencies in the final output of the two mode.
In my local environment, this modification prevents the following ui tests from failing when set parallel-compiler = true in config.toml:

    [ui] src/test\ui\associated-types\defaults-cyclic-fail-1.rs
    [ui] src/test\ui\associated-types\defaults-cyclic-fail-2.rs
    [ui] src/test\ui\associated-types\hr-associated-type-bound-2.rs
    [ui] src/test\ui\associated-types\impl-wf-cycle-1.rs
    [ui] src/test\ui\associated-types\impl-wf-cycle-2.rs
    [ui] src/test\ui\issues\issue-20413.rs
    [ui] src/test\ui\parallel_test\defaults-cyclic-fail-para.rs

rust-highfive · 2022-05-23T07:53:49Z

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

bjorn3 · 2022-05-23T08:45:40Z

compiler/rustc_data_structures/src/sync.rs

@@ -339,7 +339,10 @@ cfg_if! {
            t: T,
            for_each: impl Fn(T::Item) + Sync + Send,
        ) {
-            t.into_par_iter().for_each(for_each)
+            let ps: Vec<_> = t.into_par_iter().map(|i| catch_unwind(AssertUnwindSafe(|| for_each(i)))).collect();


Does rayon support .collect::<Result<Vec<()>, Box<dyn Any + Send>>()? That would be faster than iterating over all elements to check if any is Err I think.

We indeed need to iterate over all elements, not just know if any of them is Err. Because the err message will be output during the iteration. This is relatively consistent with the behavior in serial mode.

SparrowLii · 2022-05-24T07:25:46Z

related PR here: #68171. I think I can refer to it for a more complete fix.

lqd · 2022-05-24T07:38:06Z

@SparrowLii Does this PR fix the ICEs when using the parallel compiler with more than one thread ? (I would expect not, but you never know)

SparrowLii · 2022-05-24T09:04:31Z

I don't think this PR will solve any ICE issues, it just solves the inconsistency of parallel and non-parallel output in some scenarios.

estebank · 2022-05-24T17:58:05Z

@bors try @rust-timer queue

rust-timer · 2022-05-24T17:58:07Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-05-24T17:58:14Z

⌛ Trying commit 17483adc0ac961d5feba244218ffee7a9968f538 with merge e94ecbb4ae5fb516fe84f4b81e08b1faa86a3800...

bors · 2022-05-24T19:51:14Z

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

rust-timer · 2022-05-24T19:51:16Z

Queued e94ecbb4ae5fb516fe84f4b81e08b1faa86a3800 with parent ee9726c, future comparison URL.

rust-timer · 2022-05-24T22:07:31Z

Finished benchmarking commit (e94ecbb4ae5fb516fe84f4b81e08b1faa86a3800): comparison url.

Instruction count

Primary benchmarks: no relevant changes found
Secondary benchmarks: 🎉 relevant improvements found

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count¹	0	0	0	3	0
mean²	N/A	N/A	N/A	-1.1%	N/A
max	N/A	N/A	N/A	-1.1%	N/A

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: mixed results

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count¹	0	1	0	1	0
mean²	N/A	2.2%	N/A	-1.1%	N/A
max	N/A	2.2%	N/A	-1.1%	N/A

Cycles

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

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

number of relevant changes ↩ ↩²
the arithmetic mean of the percent change ↩ ↩²

cjgillot · 2022-06-27T19:27:21Z

Let's merge this as is, and we'll optimize if the need arises.
@bors r+ rollup=never

bors · 2022-06-27T19:27:23Z

📌 Commit ec137f2 has been approved by cjgillot

bors · 2022-06-27T19:42:12Z

⌛ Testing commit ec137f2 with merge 2f3ddd9...

bors · 2022-06-27T22:22:52Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 2f3ddd9 to master...

rust-timer · 2022-06-27T23:28:16Z

Finished benchmarking commit (2f3ddd9): comparison url.

Instruction count

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

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	4.2%	6.3%	2
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

Cycles

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

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩
number of relevant changes ↩

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 23, 2022

rust-highfive assigned estebank May 23, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2022

SparrowLii mentioned this pull request May 23, 2022

UI test failures with parallel-compiler=true #75760

Open

bjorn3 reviewed May 23, 2022

View reviewed changes

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 24, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 24, 2022

SparrowLii force-pushed the parallel branch from 17483ad to 0b0f57d Compare June 27, 2022 07:47

SparrowLii marked this pull request as draft June 27, 2022 07:50

catch unwind of every iter in parallel mode during wfcheck

ec137f2

SparrowLii force-pushed the parallel branch from 0b0f57d to ec137f2 Compare June 27, 2022 08:39

SparrowLii marked this pull request as ready for review June 27, 2022 08:40

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 Jun 27, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 27, 2022

bors merged commit 2f3ddd9 into rust-lang:master Jun 27, 2022

rustbot added this to the 1.64.0 milestone Jun 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

catch unwind in parallel mode during wfcheck #97307

catch unwind in parallel mode during wfcheck #97307

SparrowLii commented May 23, 2022 •

edited

Loading

rust-highfive commented May 23, 2022

bjorn3 May 23, 2022

SparrowLii May 23, 2022 •

edited

Loading

SparrowLii commented May 24, 2022

lqd commented May 24, 2022 •

edited

Loading

SparrowLii commented May 24, 2022 •

edited

Loading

estebank commented May 24, 2022

rust-timer commented May 24, 2022

bors commented May 24, 2022

bors commented May 24, 2022

rust-timer commented May 24, 2022

rust-timer commented May 24, 2022

cjgillot commented Jun 27, 2022

bors commented Jun 27, 2022

bors commented Jun 27, 2022

bors commented Jun 27, 2022

rust-timer commented Jun 27, 2022

catch unwind in parallel mode during wfcheck #97307

catch unwind in parallel mode during wfcheck #97307

Conversation

SparrowLii commented May 23, 2022 • edited Loading

rust-highfive commented May 23, 2022

bjorn3 May 23, 2022

Choose a reason for hiding this comment

SparrowLii May 23, 2022 • edited Loading

Choose a reason for hiding this comment

SparrowLii commented May 24, 2022

lqd commented May 24, 2022 • edited Loading

SparrowLii commented May 24, 2022 • edited Loading

estebank commented May 24, 2022

rust-timer commented May 24, 2022

bors commented May 24, 2022

bors commented May 24, 2022

rust-timer commented May 24, 2022

rust-timer commented May 24, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

cjgillot commented Jun 27, 2022

bors commented Jun 27, 2022

bors commented Jun 27, 2022

bors commented Jun 27, 2022

rust-timer commented Jun 27, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

SparrowLii commented May 23, 2022 •

edited

Loading

SparrowLii May 23, 2022 •

edited

Loading

lqd commented May 24, 2022 •

edited

Loading

SparrowLii commented May 24, 2022 •

edited

Loading