Merge `collect_mod_item_types` query into `check_well_formed` #121500

oli-obk · 2024-02-23T11:47:18Z

follow-up to #121154

this removes more potential parallel-compiler bottlenecks and moves diagnostics for the same items next to each other, instead of grouping diagnostics by analysis kind

oli-obk · 2024-02-23T11:47:27Z

@bors try @rust-timer queue

Merge `collect_mod_item_types` query into `check_well_formed` follow-up to rust-lang#121154 r? `@ghost`

bors · 2024-02-23T11:48:37Z

⌛ Trying commit 62bc4bd with merge f7e3a77...

bors · 2024-02-23T13:16:36Z

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

rust-timer · 2024-02-23T14:40:45Z

Finished benchmarking commit (f7e3a77): comparison URL.

Overall result: ✅ improvements - 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)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.7%, -0.2%]	16
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	13
All ❌✅ (primary)	-0.3%	[-0.7%, -0.2%]	16

Max RSS (memory usage)

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

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)	1.0%	[1.0%, 1.0%]	1
Regressions ❌ (secondary)	2.3%	[2.0%, 2.5%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.9%	[-5.1%, -0.8%]	4
All ❌✅ (primary)	1.0%	[1.0%, 1.0%]	1

Binary size

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

Bootstrap: 650.894s -> 650.065s (-0.13%)
Artifact size: 310.95 MiB -> 310.88 MiB (-0.02%)

oli-obk · 2024-03-07T13:44:01Z

@bors try @rust-timer queue

r? compiler

bors · 2024-03-07T13:45:12Z

⌛ Trying commit fa823ff with merge 1f106e7...

Merge `collect_mod_item_types` query into `check_well_formed` follow-up to rust-lang#121154 this removes more potential parallel-compiler bottlenecks and moves diagnostics for the same items next to each other, instead of grouping diagnostics by analysis kind

petrochenkov · 2024-03-07T14:13:33Z

compiler/rustc_middle/src/values.rs

+            && let Some(def_id) = frame.query.def_id
+        {
+            let n = tcx.generics_of(def_id).params.len();
+            vec![ty::Variance::Bivariant; n].leak()


Could you explain what happens here?
When will this impl be called? Is this logic for error recovery?

(Also tcx.arena.allocate could probably be used instead of leak.)

When will this impl be called? Is this logic for error recovery?

This is not part of recovery, but of cycle_delay_bug. In order to hide cycle errors in favor of more useful errors, we can make the query cycle turn into a span_delayed_bug. But then we need a value to return from the query, which is where all the impls in this module are invoked. They produce a dummy value for the query.

(Also tcx.arena.allocate could probably be used instead of leak.)

Then I would have to transmute the result 😆

I guess if someone is running rustc in a loop from within the same process, and keep hitting this cycle error, they would slowly lose memory

edit: leak is already being used elsewhere in this file, so this is more of a general point that we could figure out, but it seems low priority, so I think just avoiding the unsafe code is best

This is not part of recovery

I mean ErrorGuaranteed is passed to this function, so some error already happened (or at least was stashed)?
So this dummy value is never created on a good path.

Edit: the bit about span_delayed_bug is also not clear, if we have an ErrorGuaranteed then the delayed bug will never be reported.

span_delayed_bug creates an ErrorGuaranteed without emitting an error. It will ICE the compiler if you forgot to emit an error later

petrochenkov · 2024-03-07T14:15:46Z

compiler/rustc_hir_analysis/src/collect.rs

+    });
+    let _ = items.par_foreign_items(|item| {
+        Ok(CollectItemTypesVisitor { tcx }.visit_foreign_item(hir.foreign_item(item)))
+    });


In general, I'd expect having something like par_visit_item_likes_in_module instead of doing things like this here or in wf checking.

That can't be done nicely, because visit_item_likes_in_module uses a visitor. I guess we could add something that creates the visitor 4 times just like I did here, but considering after this PR we again have only a single use site of such a par_visit_item_likes_in_module method, keeping it inline at the one use site seems best to me?

considering after this PR we again have only a single use site of such a par_visit_item_likes_in_module method

If overhead of parallel queries was low enough, then I'd expect pretty much every visit_item_likes_in_modules and visit_all_item_likes_in_crates to benefit from being turned into its par_ version.
(Although maybe we'll need a special Par version of visitor for this.)

I'll add it to the list of things I am investigating around the parallel compiler

petrochenkov · 2024-03-07T21:06:08Z

No requests, besides the general questions I was also waiting for perf results from #121500 (comment), that perf run is still in the queue.

oli-obk · 2024-03-07T21:11:53Z

whoops, force pushes for clippy ui tests cancelled rust-timer, but not the try build, so reusing that build

@rust-timer build 1f106e7

rust-timer · 2024-03-08T01:09:42Z

Finished benchmarking commit (1f106e7): comparison URL.

Overall result: ✅ improvements - 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)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.7%, -0.2%]	3
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.2%]	9
All ❌✅ (primary)	-0.5%	[-0.7%, -0.2%]	3

Max RSS (memory usage)

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

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.6%	[2.4%, 2.7%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 648.501s -> 647.819s (-0.11%)
Artifact size: 172.69 MiB -> 172.65 MiB (-0.02%)

petrochenkov · 2024-03-08T09:06:55Z

@bors r+

bors · 2024-03-08T09:06:58Z

📌 Commit ae50e36 has been approved by petrochenkov

It is now in the queue for this repository.

Merge `collect_mod_item_types` query into `check_well_formed` follow-up to rust-lang#121154 this removes more potential parallel-compiler bottlenecks and moves diagnostics for the same items next to each other, instead of grouping diagnostics by analysis kind

bors · 2024-03-08T12:06:29Z

⌛ Testing commit ae50e36 with merge 5bff44f...

bors · 2024-03-08T12:10:32Z

💔 Test failed - checks-actions

rust-log-analyzer · 2024-03-08T12:11:00Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

  SDKROOT: /Applications/Xcode_14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk
  AR: ar
##[endgroup]
curl: (28) Operation too slow. Less than 100 bytes/sec transferred the last 5 seconds
Error: Failure while executing; `/usr/bin/env /usr/local/Homebrew/Library/Homebrew/shims/shared/curl --disable --cookie /dev/null --globoff --user-agent Homebrew/4.2.9\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 13.6.4\)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --remote-time --output /Users/runner/Library/Caches/Homebrew/api/formula.jws.json --location --disable --cookie /dev/null --globoff --show-error --user-agent Homebrew/4.2.9\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 13.6.4\)\ curl/8.4.0 --header Accept-Language:\ en --fail --progress-bar --silent --compressed --speed-limit 100 --speed-time 5 https://formulae.brew.sh/api/formula.jws.json` exited with 28. Here's the output:

##[error]Process completed with exit code 1.
Post job cleanup.
[command]/usr/local/bin/git version

oli-obk · 2024-03-08T12:20:46Z

@bors retry curl: (28) Operation too slow. Less than 100 bytes/sec transferred the last 5 seconds

bors · 2024-03-08T15:06:39Z

⌛ Testing commit ae50e36 with merge 74acabe...

bors · 2024-03-08T17:29:52Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 74acabe to master...

Run a single huge par_body_owners instead of many small ones after each other. This improves parallel rustc parallelism by avoiding the bottleneck after each individual `par_body_owners` (because it needs to wait for queries to finish, so if there is one long running one, a lot of cores will be idle while waiting for the single query). based on rust-lang#121500

rust-timer · 2024-03-08T19:42:57Z

Finished benchmarking commit (74acabe): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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)	-	-	0
Improvements ✅ (primary)	-0.6%	[-0.8%, -0.2%]	4
Improvements ✅ (secondary)	-0.5%	[-0.6%, -0.5%]	2
All ❌✅ (primary)	-0.6%	[-0.8%, -0.2%]	4

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.6%	[-5.8%, -5.4%]	2
All ❌✅ (primary)	-	-	0

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)	1.7%	[1.4%, 2.0%]	3
Regressions ❌ (secondary)	2.0%	[1.7%, 2.4%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-3.3%, -2.1%]	5
All ❌✅ (primary)	1.7%	[1.4%, 2.0%]	3

Binary size

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

Bootstrap: 649.016s -> 648.792s (-0.03%)
Artifact size: 172.57 MiB -> 172.56 MiB (-0.01%)

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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 23, 2024

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 Feb 23, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024

Auto merge of rust-lang#121500 - oli-obk:track_errors12, r=<try>

f7e3a77

Merge `collect_mod_item_types` query into `check_well_formed` follow-up to rust-lang#121154 r? `@ghost`

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 Feb 23, 2024

oli-obk added 3 commits March 7, 2024 12:42

Collect mod item types in parallel, just like wfcheck

de3fb8d

Use the same collection order as check_mod_type_wf

ebf1b92

Remove redundant query invocations in rustdoc

42ab88d

oli-obk force-pushed the track_errors12 branch from 62bc4bd to fa823ff Compare March 7, 2024 13:43

This comment has been minimized.

Sign in to view

rustbot assigned petrochenkov Mar 7, 2024

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 7, 2024

petrochenkov reviewed Mar 7, 2024

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 7, 2024

This comment has been minimized.

Sign in to view

Merge collect_mod_item_types query into check_well_formed

ae50e36

oli-obk force-pushed the track_errors12 branch from fa823ff to ae50e36 Compare March 7, 2024 14:26

oli-obk mentioned this pull request Mar 7, 2024

Run a single huge par_body_owners instead of many small ones after each other. #122140

Merged

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 Mar 8, 2024

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 Mar 8, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 8, 2024

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 Mar 8, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 8, 2024

bors merged commit 74acabe into rust-lang:master Mar 8, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Mar 8, 2024

This was referenced Mar 8, 2024

When displaying multispans, ignore empty lines adjacent to ... #122029

Merged

Stabilize associated type bounds (RFC 2289) #122055

Merged

Better comment for implicit captures in RPITIT #122100

Merged

oli-obk deleted the track_errors12 branch March 8, 2024 19:45

lqd mentioned this pull request Dec 10, 2024

Hang from infinitely-self-recursive trait alias #133901

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge `collect_mod_item_types` query into `check_well_formed` #121500

Merge `collect_mod_item_types` query into `check_well_formed` #121500

oli-obk commented Feb 23, 2024 •

edited

Loading

oli-obk commented Feb 23, 2024

This comment has been minimized.

bors commented Feb 23, 2024

This comment has been minimized.

bors commented Feb 23, 2024

This comment has been minimized.

rust-timer commented Feb 23, 2024

oli-obk commented Mar 7, 2024

This comment has been minimized.

bors commented Mar 7, 2024

petrochenkov Mar 7, 2024

oli-obk Mar 7, 2024

oli-obk Mar 7, 2024

petrochenkov Mar 7, 2024 •

edited

Loading

oli-obk Mar 7, 2024

petrochenkov Mar 7, 2024

oli-obk Mar 7, 2024

petrochenkov Mar 7, 2024 •

edited

Loading

oli-obk Mar 7, 2024

This comment has been minimized.

petrochenkov commented Mar 7, 2024

oli-obk commented Mar 7, 2024

This comment has been minimized.

rust-timer commented Mar 8, 2024

petrochenkov commented Mar 8, 2024

bors commented Mar 8, 2024

bors commented Mar 8, 2024

bors commented Mar 8, 2024

rust-log-analyzer commented Mar 8, 2024

oli-obk commented Mar 8, 2024

bors commented Mar 8, 2024

bors commented Mar 8, 2024

rust-timer commented Mar 8, 2024

Merge collect_mod_item_types query into check_well_formed #121500

Merge collect_mod_item_types query into check_well_formed #121500

Conversation

oli-obk commented Feb 23, 2024 • edited Loading

oli-obk commented Feb 23, 2024

This comment has been minimized.

bors commented Feb 23, 2024

This comment has been minimized.

bors commented Feb 23, 2024

This comment has been minimized.

rust-timer commented Feb 23, 2024

Overall result: ✅ improvements - no action needed

oli-obk commented Mar 7, 2024

This comment has been minimized.

bors commented Mar 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrochenkov Mar 7, 2024 • edited Loading

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

petrochenkov Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

petrochenkov commented Mar 7, 2024

oli-obk commented Mar 7, 2024

This comment has been minimized.

rust-timer commented Mar 8, 2024

Overall result: ✅ improvements - no action needed

petrochenkov commented Mar 8, 2024

bors commented Mar 8, 2024

bors commented Mar 8, 2024

bors commented Mar 8, 2024

rust-log-analyzer commented Mar 8, 2024

oli-obk commented Mar 8, 2024

bors commented Mar 8, 2024

bors commented Mar 8, 2024

rust-timer commented Mar 8, 2024

Overall result: ✅ improvements - no action needed

Merge `collect_mod_item_types` query into `check_well_formed` #121500

Merge `collect_mod_item_types` query into `check_well_formed` #121500

oli-obk commented Feb 23, 2024 •

edited

Loading

petrochenkov Mar 7, 2024 •

edited

Loading

petrochenkov Mar 7, 2024 •

edited

Loading