Fewer global node_id_to_def_id lookups by oli-obk · Pull Request #156173 · rust-lang/rust

oli-obk · 2026-05-05T05:06:26Z

Several of these are unnecessary if we track the LocalDefId together with the NodeId. We can't remove the NodeId entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future

r? @petrochenkov

petrochenkov · 2026-05-05T11:14:57Z

@bors try @rust-timer queue

Fewer global node_id_to_def_id lookups

petrochenkov · 2026-05-05T11:15:24Z

r=me if there's no perf regressions.

rust-bors · 2026-05-05T13:23:55Z

☀️ Try build successful (CI)
Build commit: 4d34368 (4d34368fecac63a25397b0cb35a8968dd78945cd, parent: 4feb7221f4d445120a5061b16ce7222adbfdf6f6)

rust-timer · 2026-05-05T15:13:18Z

Finished benchmarking commit (4d34368): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.3%, -0.2%]	9
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.2%]	4
All ❌✅ (primary)	-0.2%	[-0.3%, -0.2%]	9

Max RSS (memory usage)

Results (primary -2.2%, secondary -2.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.1%	[0.5%, 2.2%]	3
Improvements ✅ (primary)	-2.2%	[-2.2%, -2.2%]	2
Improvements ✅ (secondary)	-4.6%	[-7.8%, -1.9%]	7
All ❌✅ (primary)	-2.2%	[-2.2%, -2.2%]	2

Cycles

Results (primary -2.1%, secondary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.3%	[0.4%, 2.6%]	6
Improvements ✅ (primary)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	1
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 495.518s -> 501.103s (1.13%)
Artifact size: 394.42 MiB -> 394.41 MiB (-0.00%)

petrochenkov · 2026-05-05T15:32:13Z

@bors r+ rollup=maybe
The libc-0.2.172 and unused-warnings improvements should be easily recognizable in a rollup.

rust-bors · 2026-05-05T15:32:16Z

📌 Commit 0a151ca has been approved by petrochenkov

It is now in the queue for this repository.

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

…id` instead

rustbot · 2026-05-06T09:01:08Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

oli-obk · 2026-05-06T09:02:20Z

@bors r=petrochenkov

fixed rebase conflicts
added a doc comment
debug print the new DefId fields

rust-bors · 2026-05-06T09:02:22Z

📌 Commit a082567 has been approved by petrochenkov

It is now in the queue for this repository.

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

…uwer Rollup of 7 pull requests Successful merges: - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #156173 (Fewer global node_id_to_def_id lookups) - #155961 (Deny warnings in the test for crates that are available on stable) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156141 (Resolve some cases of #132279 by using the right typing mode in the next solver) - #156202 (llvm: Use correct type for splat mask)

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

…uwer Rollup of 7 pull requests Successful merges: - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #149509 (Lint unused pub items in binary crates) - #156173 (Fewer global node_id_to_def_id lookups) - #155961 (Deny warnings in the test for crates that are available on stable) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156202 (llvm: Use correct type for splat mask) Failed merges: - #156236 (resolve: Remove `MacroData`)

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

…uwer Rollup of 9 pull requests Successful merges: - #156061 (Support `-Cpanic=unwind` on WASI targets) - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #149509 (Lint unused pub items in binary crates) - #156173 (Fewer global node_id_to_def_id lookups) - #155961 (Deny warnings in the test for crates that are available on stable) - #155981 (Use special DefIds for aliases) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156202 (llvm: Use correct type for splat mask) Failed merges: - #156236 (resolve: Remove `MacroData`)

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

Rollup of 14 pull requests Successful merges: - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #149509 (Lint unused pub items in binary crates) - #156173 (Fewer global node_id_to_def_id lookups) - #156177 (windows/time: avoid being too close to 0) - #155961 (Deny warnings in the test for crates that are available on stable) - #155981 (Use special DefIds for aliases) - #156109 (Migrate libraries from ptr::slice_from_raw_parts to .cast_slice) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156202 (llvm: Use correct type for splat mask) - #156227 (Add Trusty OS to the generic error implementation.) - #156237 (compiletest: prevent directives from having multiple revisions) - #156241 (Move tests coercion) - #156258 (Document wasi-sdk minimum versions for WASI targets)

@petrochenkov

…trochenkov Fewer global node_id_to_def_id lookups Several of these are unnecessary if we track the `LocalDefId` together with the `NodeId`. We can't remove the `NodeId` entirely, as it is needed for lints, but it's a useful refactoring for splitting node_id_to_def_id into a per-owner table in the future r? @petrochenkov

rustbot assigned petrochenkov May 5, 2026

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 May 5, 2026

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 May 5, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request May 5, 2026

Auto merge of #156173 - oli-obk:fewer-global-lookups, r=<try>

4d34368

Fewer global node_id_to_def_id lookups

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 5, 2026

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 May 5, 2026

rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 5, 2026

jhpratt mentioned this pull request May 5, 2026

Rollup of 12 pull requests #156207

Closed

rust-bors Bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 5, 2026

This comment has been minimized.

Sign in to view

cjgillot reviewed May 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/imports.rs

Comment thread compiler/rustc_resolve/src/lib.rs

oli-obk added 4 commits May 6, 2026 10:58

Avoid some global node_id_to_def_id lookups in import resolution

0afe083

Avoid using id followed by local_def_id if we can just call `def_…

191cda5

…id` instead

Avoid hitting the global node_id_to_def_id table for unused macros

400240a

Follow-up cleanups reusing the now-available LocalDefIds

a082567

oli-obk force-pushed the fewer-global-lookups branch from 0a151ca to a082567 Compare May 6, 2026 09:01

rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 6, 2026

petrochenkov mentioned this pull request May 6, 2026

resolve: Try removing questionable optimizations in eff vis computation #156185

Open

JonathanBrouwer mentioned this pull request May 6, 2026

Rollup of 8 pull requests #156232

Closed

JonathanBrouwer mentioned this pull request May 6, 2026

Rollup of 7 pull requests #156233

Closed

JonathanBrouwer mentioned this pull request May 6, 2026

Rollup of 7 pull requests #156238

Closed

JonathanBrouwer mentioned this pull request May 6, 2026

Rollup of 7 pull requests #156249

Closed

JonathanBrouwer mentioned this pull request May 6, 2026

Rollup of 9 pull requests #156254

Closed

jhpratt mentioned this pull request May 7, 2026

Rollup of 11 pull requests #156261

Closed

jhpratt mentioned this pull request May 7, 2026

Rollup of 11 pull requests #156262

Closed

jhpratt mentioned this pull request May 7, 2026

Rollup of 14 pull requests #156263

Closed

jhpratt mentioned this pull request May 7, 2026

Rollup of 12 pull requests #156267

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fewer global node_id_to_def_id lookups#156173

Fewer global node_id_to_def_id lookups#156173
oli-obk wants to merge 4 commits intorust-lang:mainfrom
oli-obk:fewer-global-lookups

oli-obk commented May 5, 2026

Uh oh!

petrochenkov commented May 5, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

petrochenkov commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

This comment has been minimized.

rust-timer commented May 5, 2026

Uh oh!

petrochenkov commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

rustbot commented May 6, 2026

Uh oh!

oli-obk commented May 6, 2026

Uh oh!

rust-bors Bot commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

oli-obk commented May 5, 2026

Uh oh!

petrochenkov commented May 5, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

petrochenkov commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

This comment has been minimized.

rust-timer commented May 5, 2026

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

petrochenkov commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

rustbot commented May 6, 2026

Uh oh!

oli-obk commented May 6, 2026

Uh oh!

rust-bors Bot commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants