definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex` by aerooneqq · Pull Request #156963 · rust-lang/rust

aerooneqq · 2026-05-26T10:42:27Z

View all comments

This PR removes DefPathTable and uses LocalDefId instead of DefIndex where possible.

r? @petrochenkov

petrochenkov · 2026-05-26T10:51:13Z

@bors try @rust-timer queue

definitions: remove `DefPathTable`, merge `index_to_key` and `def_path_hashes`

aerooneqq · 2026-05-26T15:01:01Z

Maybe need to restart perf, there were problems with github actions.
cc @petrochenkov

petrochenkov · 2026-05-26T15:47:32Z

@bors try cancel

rust-bors · 2026-05-26T15:47:37Z

Try build cancelled. Cancelled workflows:

petrochenkov · 2026-05-26T15:47:54Z

@bors try @rust-timer queue

definitions: remove `DefPathTable`, merge `index_to_key` and `def_path_hashes`

rust-bors · 2026-05-26T17:59:09Z

☀️ Try build successful (CI)
Build commit: 29d6359 (29d635947db7a31f5b2a840c1b2dab059caebd46, parent: b7e97a98f24cdb1335850209991455cb9734d333)

rust-timer · 2026-05-26T19:04:49Z

Finished benchmarking commit (29d6359): comparison URL.

Overall result: ❌ regressions - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@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.3%	[0.0%, 0.8%]	21
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 3.3%, secondary -0.9%)

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

	mean	range	count
Regressions ❌ (primary)	3.3%	[3.3%, 3.3%]	1
Regressions ❌ (secondary)	2.1%	[1.3%, 3.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.0%	[-6.4%, -1.5%]	2
All ❌✅ (primary)	3.3%	[3.3%, 3.3%]	1

Cycles

Results (primary 2.1%, secondary -2.2%)

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

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

Binary size

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

Bootstrap: 511.751s -> 510.109s (-0.32%)
Artifact size: 400.67 MiB -> 400.65 MiB (-0.00%)

petrochenkov · 2026-05-27T09:47:17Z

The merge of IndexVec<DefIndex, Hash64> and IndexVec<DefIndex, DefKey> into IndexVec<DefIndex, (DefKey, Hash64)> is probably causing the minor regressions above.
Perhaps due to extra padding in (DefKey, Hash64)?
IndexVecs are essentially Vecs, they are laid out in memory effectively, the indexing is basically free, so merging for performance reasons doesn't make much sense.

As I understand the goal was to later convert the IndexVecs to HashMaps to support holes in DefIds for #156798.
As this PR shows the table accesses here are performance sensitive, so that probably won't fly, and #156798 will need to use synthetic HIR/etc for the removed code, and keep the DefId range continuous.

The rest of the refactoring looks good to me, we don't need two structures there, one is enough.

aerooneqq · 2026-05-28T07:02:42Z

Let's do another perf run after index_to_key and def_path_hashes are returned.
@rustbot ready

petrochenkov · 2026-05-29T12:39:15Z

r=me after addressing #156963 (comment) and squashing commits.

…sible

aerooneqq · 2026-05-29T12:52:52Z

@rustbot ready

petrochenkov · 2026-05-29T13:00:53Z

@bors r+ rollup=maybe

rust-bors · 2026-05-29T13:00:57Z

📌 Commit 7767295 has been approved by petrochenkov

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.

@petrochenkov

…ochenkov definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex` This PR removes `DefPathTable` and uses `LocalDefId` instead of `DefIndex` where possible. r? @petrochenkov

…uwer Rollup of 9 pull requests Successful merges: - #156963 (definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex`) - #157053 (Eagerly resolve delegations in late resolution) - #157068 (NVPTX: Remove the unstable ptx linker flavor) - #157076 (Various proc-macro related code cleanups) - #157100 (Some more per owner things) - #157065 (Stabilize `Path::is_empty`) - #157070 (Remove `skip_arg` attribute from `Diagnostic` and `Subdiagnostic` proc-macros) - #157088 (Improve suggestions for malformed deprecated attribute) - #157103 (Add reproducibly failing tests for parallel frontend)

@petrochenkov

…ochenkov definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex` This PR removes `DefPathTable` and uses `LocalDefId` instead of `DefIndex` where possible. r? @petrochenkov

…uwer Rollup of 11 pull requests Successful merges: - #156960 (Some cleanups around passing extra lifetime params from the resolver to ast lowering) - #156963 (definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex`) - #157053 (Eagerly resolve delegations in late resolution) - #157068 (NVPTX: Remove the unstable ptx linker flavor) - #157076 (Various proc-macro related code cleanups) - #157106 (add ABI check logic for wasm) - #154835 (std::offload sharedmem) - #157065 (Stabilize `Path::is_empty`) - #157070 (Remove `skip_arg` attribute from `Diagnostic` and `Subdiagnostic` proc-macros) - #157088 (Improve suggestions for malformed deprecated attribute) - #157103 (Add reproducibly failing tests for parallel frontend) Failed merges: - #157100 (Some more per owner things)

@petrochenkov

…ochenkov definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex` This PR removes `DefPathTable` and uses `LocalDefId` instead of `DefIndex` where possible. r? @petrochenkov

…uwer Rollup of 11 pull requests Successful merges: - #149195 (resolve: Partially convert `ambiguous_glob_imports` lint into a hard error) - #156960 (Some cleanups around passing extra lifetime params from the resolver to ast lowering) - #156963 (definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex`) - #157053 (Eagerly resolve delegations in late resolution) - #157068 (NVPTX: Remove the unstable ptx linker flavor) - #157076 (Various proc-macro related code cleanups) - #157106 (add ABI check logic for wasm) - #154835 (std::offload sharedmem) - #157065 (Stabilize `Path::is_empty`) - #157088 (Improve suggestions for malformed deprecated attribute) - #157103 (Add reproducibly failing tests for parallel frontend) Failed merges: - #157100 (Some more per owner things)

…uwer Rollup of 16 pull requests Successful merges: - #149195 (resolve: Partially convert `ambiguous_glob_imports` lint into a hard error) - #156960 (Some cleanups around passing extra lifetime params from the resolver to ast lowering) - #156963 (definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex`) - #157053 (Eagerly resolve delegations in late resolution) - #157068 (NVPTX: Remove the unstable ptx linker flavor) - #157076 (Various proc-macro related code cleanups) - #157106 (add ABI check logic for wasm) - #154835 (std::offload sharedmem) - #157065 (Stabilize `Path::is_empty`) - #157088 (Improve suggestions for malformed deprecated attribute) - #157098 (Add the `clflushopt` x86 target feature) - #157103 (Add reproducibly failing tests for parallel frontend) - #157111 (Update target maintainer for x86_64-unknown-linux-none) - #157116 (rustc_public: add `with_cx()` to `CompilerInterface`) - #157119 (ast_lowering: Simplify `resolve_pin_drop_sugar_impl_item`) - #157120 (Cleanups around attribute target checking) Failed merges: - #157100 (Some more per owner things)

@petrochenkov

Rollup merge of #156963 - aerooneqq:defs-refactoring, r=petrochenkov definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex` This PR removes `DefPathTable` and uses `LocalDefId` instead of `DefIndex` where possible. r? @petrochenkov

rustbot assigned petrochenkov May 26, 2026

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 26, 2026

aerooneqq marked this pull request as ready for review May 26, 2026 10:48

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 26, 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 26, 2026

This comment has been minimized.

Sign in to view

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

Auto merge of #156963 - aerooneqq:defs-refactoring, r=<try>

6349cc5

definitions: remove `DefPathTable`, merge `index_to_key` and `def_path_hashes`

petrochenkov reviewed May 26, 2026

View reviewed changes

Comment thread compiler/rustc_middle/src/ty/context.rs Outdated

Comment thread compiler/rustc_hir/src/definitions.rs

Comment thread compiler/rustc_metadata/src/rmeta/encoder.rs Outdated

Comment thread compiler/rustc_hir/src/definitions.rs Outdated

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

This comment has been minimized.

Sign in to view

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

Auto merge of #156963 - aerooneqq:defs-refactoring, r=<try>

29d6359

definitions: remove `DefPathTable`, merge `index_to_key` and `def_path_hashes`

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 26, 2026

cjgillot reviewed May 27, 2026

View reviewed changes

Comment thread compiler/rustc_hir/src/definitions.rs

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 28, 2026

Remove DefPathTable, use LocalDefId instead of DefIndex where pos…

7767295

…sible

aerooneqq force-pushed the defs-refactoring branch from aa3fc79 to 7767295 Compare May 29, 2026 12:51

aerooneqq changed the title ~~definitions: remove DefPathTable, merge index_to_key and def_path_hashes~~ definitions: remove DefPathTable, use LocalDefId instead of DefIndex May 29, 2026

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

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-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2026

JonathanBrouwer mentioned this pull request May 29, 2026

Rollup of 9 pull requests #157109

Closed

JonathanBrouwer mentioned this pull request May 29, 2026

Rollup of 11 pull requests #157113

Closed

JonathanBrouwer mentioned this pull request May 29, 2026

Rollup of 11 pull requests #157114

Closed

JonathanBrouwer mentioned this pull request May 29, 2026

Rollup of 16 pull requests #157121

Merged

rust-bors Bot merged commit b8d9ef8 into rust-lang:main May 30, 2026
12 checks passed

rustbot added this to the 1.98.0 milestone May 30, 2026

Uh oh!

Conversation

aerooneqq commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented May 26, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aerooneqq commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented May 26, 2026

Uh oh!

rust-bors Bot commented May 26, 2026

Uh oh!

petrochenkov commented May 26, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented May 26, 2026

Uh oh!

This comment has been minimized.

rust-timer commented May 26, 2026

Overall result: ❌ regressions - please read:

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

petrochenkov commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aerooneqq commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented May 29, 2026

Uh oh!

aerooneqq commented May 29, 2026

Uh oh!

petrochenkov commented May 29, 2026

Uh oh!

rust-bors Bot commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aerooneqq commented May 26, 2026 •

edited

Loading

aerooneqq commented May 26, 2026 •

edited

Loading

petrochenkov commented May 27, 2026 •

edited

Loading

aerooneqq commented May 28, 2026 •

edited

Loading