Don't opt_rpitit_info as a separate query

[compiler-errors]

... another attempt to undo regressions

r? @ghost

[compiler-errors]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 80e963b35d9173ab99b014f5c2e9b3556ba63e68 with merge 84675a5b3bd77db9e6142ec6a6756f3e346332e7...

[bors]

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

[rust-timer]

Finished benchmarking commit (84675a5b3bd77db9e6142ec6a6756f3e346332e7): 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.6%	[-1.1%, -0.3%]	79
Improvements ✅ (secondary)	-1.2%	[-2.8%, -0.2%]	40
All ❌✅ (primary)	-0.6%	[-1.1%, -0.3%]	79

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)	2.4%	[1.8%, 3.0%]	2
Regressions ❌ (secondary)	2.7%	[2.0%, 3.4%]	2
Improvements ✅ (primary)	-2.1%	[-3.0%, -0.8%]	7
Improvements ✅ (secondary)	-3.9%	[-8.9%, -0.9%]	6
All ❌✅ (primary)	-1.1%	[-3.0%, 3.0%]	9

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)	-	-	0
Improvements ✅ (primary)	-2.1%	[-2.8%, -1.1%]	7
Improvements ✅ (secondary)	-3.9%	[-5.3%, -2.0%]	12
All ❌✅ (primary)	-2.1%	[-2.8%, -1.1%]	7

[compiler-errors]

I think this helps with most of the regressions in #108700. I can fix that FIXME in a follow-up.

r? @spastorino cc @nnethercote

[spastorino]

In general the approach looks good to me.

[spastorino]

I'm not sure what's makes the opt_rpitit_info query be fired that much, if it's this call or the other one (because there are just 2), but this call could be the one. I was wondering, what if we make find_by_def_id called below return None for rpitits and the code on line 256 would just be ...

self.live_symbols.insert(id);

if let Some(node) = self.tcx.hir().find_by_def_id(id) {
    self.visit_node(node);
}

I'm not sure if there's any other situation where find_by_def_id returns None and we don't want to do self.live_symbols.insert(id), though.

[spastorino]

With this patch ...

diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
index 746cf488589..e76de89e432 100644
--- a/compiler/rustc_middle/src/hir/map/mod.rs
+++ b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -316,7 +316,7 @@ pub fn find(self, id: HirId) -> Option<Node<'hir>> {
     /// Retrieves the `Node` corresponding to `id`, returning `None` if cannot be found.
     #[inline]
     pub fn find_by_def_id(self, id: LocalDefId) -> Option<Node<'hir>> {
-        self.find(self.local_def_id_to_hir_id(id))
+        self.find(self.tcx.opt_local_def_id_to_hir_id(id)?)
     }

     /// Retrieves the `Node` corresponding to `id`, panicking if it cannot be found.
diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
index 73ee51d5f3a..1d713bbd055 100644
--- a/compiler/rustc_passes/src/dead.rs
+++ b/compiler/rustc_passes/src/dead.rs
@@ -243,18 +243,12 @@ fn mark_live_symbols(&mut self) {
                 continue;
             }

-            // Avoid accessing the HIR for the synthesized associated type generated for RPITITs.
-            if self.tcx.opt_rpitit_info(id).is_some() {
-                self.live_symbols.insert(id);
-                continue;
-            }
-
             // in the case of tuple struct constructors we want to check the item, not the generated
             // tuple struct constructor function
             let id = self.struct_constructors.get(&id).copied().unwrap_or(id);

+            self.live_symbols.insert(id);
             if let Some(node) = self.tcx.hir().find_by_def_id(id) {
-                self.live_symbols.insert(id);
                 self.visit_node(node);
             }
         }

all the tests pass. May worth running perf on this?.

[compiler-errors]

That change seems orthogonal to this one -- this PR still demonstrates that we don't need opt_rpitit_info as a query, which is a change I want anyways because storing it in the AssocTy just seems cleaner.

[compiler-errors]

To be clear, if we keep opt_rpitit_info around, other PRs may add other calls to the query that may cause regressions. Let's avoid this by fixing the problem at its source. This optimization may also be helpful, feel free to test it if you want, but I'm gonna land this PR I think.

if it's this call or the other one (because there are just 2)

There are 5 calls to opt_rpitit_info that I can see in the codebase.

[spastorino]

@compiler-errors r=me when you decide what to do with the rest of the comments :) but whatever you decide is fine by me.

[compiler-errors]

@bors r=spastorino

[bors]

📌 Commit cbd8ea01a77d0aefc8eddb89203105f3ee661acf has been approved by spastorino

It is now in the queue for this repository.

[compiler-errors]

Few more comments

@bors r=spastorino

[bors]

📌 Commit ce8dae5 has been approved by spastorino

It is now in the queue for this repository.

[bors]

⌛ Testing commit ce8dae5 with merge 0058748...

[bors]

☀️ Test successful - checks-actions
Approved by: spastorino
Pushing 0058748 to master...

[rust-timer]

Finished benchmarking commit (0058748): 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.5%	[-1.1%, -0.2%]	86
Improvements ✅ (secondary)	-1.2%	[-2.7%, -0.2%]	38
All ❌✅ (primary)	-0.5%	[-1.1%, -0.2%]	86

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.9%	[1.3%, 2.5%]	2
Regressions ❌ (secondary)	1.5%	[1.5%, 1.5%]	1
Improvements ✅ (primary)	-1.9%	[-3.1%, -0.6%]	10
Improvements ✅ (secondary)	-2.4%	[-5.9%, -1.3%]	11
All ❌✅ (primary)	-1.3%	[-3.1%, 2.5%]	12

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.4%	[2.2%, 2.7%]	3
Improvements ✅ (primary)	-2.2%	[-2.9%, -0.9%]	9
Improvements ✅ (secondary)	-3.5%	[-4.8%, -2.1%]	12
All ❌✅ (primary)	-2.2%	[-2.9%, -0.9%]	9