Skip to content

Deterministic encoding of DefPathHashMap - #162910

Merged
rust-bors[bot] merged 6 commits into
rust-lang:mainfrom
aerooneqq:det-def-path-hash-map-encoding
Sep 28, 2026
Merged

rust-bors[bot] merged 6 commits into
rust-lang:mainfrom
aerooneqq:det-def-path-hash-map-encoding

Conversation

@aerooneqq

@aerooneqq aerooneqq commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

View all comments

This PR splits DefPathHashMap into two parts: deterministic (det_part) and non-deterministic (non_det_part). det_part is used while we sure that allocation and insertion order of def ids is deterministic, at the moment of writing it happens before we start parallel checks after prefetch of hir_crate_items in run_required_analysis. Up until this point of compilation the allocation of def ids and their insertion order into det_part should be the same between different compilations.

Next, when non-determinism starts due to parallel compilation we put all mapping between local hash and def ids into a SortedMap which gives us ready-to-use sorted by local hash slice of pairs to encode while encoding metadata.

Note, that this PR does not solve the problem of allocation of different def indices to same code entities (meaning same local hash), this problem is solved in #162809 by remapping needed local def indices.
Also note that we serialize det_part as a raw bytes sequence, so during remapping if we do not place all def indices that are needed to be remapped into separate container we will end up with copying det_part, changing some entries in it and then serialize it as a bytes sequence. With the approach in this PR we do not copy and modify det_part, instead we do all modifications in non_det_part.

r? @petrochenkov

@rustbot 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 Sep 17, 2026
@aerooneqq
aerooneqq force-pushed the det-def-path-hash-map-encoding branch from 11ec6a0 to 417b0b5 Compare September 17, 2026 15:13
@petrochenkov

Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 17, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Sep 17, 2026
…try>

Deterministic encoding of `DefPathHashMap`
@rust-bors

rust-bors Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: df2b5a2 (df2b5a2f0e02a8844635d39578b4c1b338a8c0d0)
Base parent: c999cef (c999cef531ea9059e189e82fe0e82c5daf249bc9)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (df2b5a2): comparison URL.

Overall result: ❌✅ regressions and improvements - 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 rustc-perf
@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.5% [0.1%, 0.8%] 8
Regressions ❌
(secondary)
0.4% [0.1%, 0.9%] 33
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.4% [-0.9%, -0.3%] 9
All ❌✅ (primary) 0.5% [0.1%, 0.8%] 8

Max RSS (memory usage)

Results (primary 3.1%, secondary 4.7%)

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

mean range count
Regressions ❌
(primary)
3.1% [0.7%, 12.2%] 195
Regressions ❌
(secondary)
4.8% [0.5%, 12.7%] 307
Improvements ✅
(primary)
-0.8% [-0.8%, -0.8%] 1
Improvements ✅
(secondary)
-3.1% [-3.1%, -3.1%] 1
All ❌✅ (primary) 3.1% [-0.8%, 12.2%] 196

Cycles

Results (primary 10.3%, secondary 4.2%)

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

mean range count
Regressions ❌
(primary)
10.3% [2.8%, 17.3%] 4
Regressions ❌
(secondary)
4.4% [2.0%, 10.2%] 36
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% [-3.1%, -3.1%] 1
All ❌✅ (primary) 10.3% [2.8%, 17.3%] 4

Binary size

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

Bootstrap: 500.132s -> 496.691s (-0.69%)
Artifact size: 406.81 MiB -> 409.64 MiB (0.70%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Sep 17, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 18, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 18, 2026
…try>

Deterministic encoding of `DefPathHashMap`
@rust-bors

This comment has been minimized.

@rust-bors

rust-bors Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: a367041 (a36704158886f7496c764811537d3ff5babcbaf9)
Base parent: 420ed2a (420ed2a0c3d7225b1744266fd884d431b4d8cfe0)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (a367041): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@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.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 3.6%)

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

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

Cycles

Results (secondary 3.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)
3.1% [2.5%, 4.2%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (primary -0.1%, secondary -0.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)
- - 0
Improvements ✅
(primary)
-0.1% [-0.2%, -0.0%] 67
Improvements ✅
(secondary)
-0.1% [-0.2%, -0.0%] 53
All ❌✅ (primary) -0.1% [-0.2%, -0.0%] 67

Bootstrap: 498.333s -> 497.372s (-0.19%)
Artifact size: 408.93 MiB -> 408.87 MiB (-0.02%)

@rustbot rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Sep 18, 2026
@aerooneqq
aerooneqq force-pushed the det-def-path-hash-map-encoding branch from a07e80c to 17afd99 Compare September 21, 2026 11:44
@aerooneqq
aerooneqq force-pushed the det-def-path-hash-map-encoding branch from 17afd99 to 0dde918 Compare September 21, 2026 11:49
@aerooneqq
aerooneqq marked this pull request as ready for review September 21, 2026 12:30
@rustbot

rustbot commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 21, 2026
@rustbot rustbot 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 Sep 24, 2026
@rustbot

rustbot commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@aerooneqq

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot 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 Sep 25, 2026
Comment thread compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs Outdated
Comment thread compiler/rustc_hir_id/src/definitions.rs Outdated
Comment thread compiler/rustc_hir_id/src/definitions.rs Outdated
@petrochenkov 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 Sep 25, 2026
@rustbot 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 Sep 28, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 2f77d20 has been approved by petrochenkov

It is now in the queue for this repository.

@rust-bors 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 Sep 28, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

@bors rollup=maybe

rust-bors Bot pushed a commit that referenced this pull request Sep 28, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #163327 (remove `MutTy`)
 - #162910 (Deterministic encoding of `DefPathHashMap`)
 - #163009 (dont store arbitrary parsed attributes in thir)
 - #162683 (Use attribute parser for `#[inline()]` attribute check)
 - #163429 (lint on `Ident::from_str_and_span` taking a string literal)
 - #163431 (Allow `#[repr(simd)]` with `f16b`)
 - #163433 (Support also `try-jobs:` to specify custom try jobs)
@rust-bors
rust-bors Bot merged commit 2cc3eeb into rust-lang:main Sep 28, 2026
13 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Sep 28, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 28, 2026
Rollup merge of #162910 - aerooneqq:det-def-path-hash-map-encoding, r=petrochenkov

Deterministic encoding of `DefPathHashMap`

This PR splits DefPathHashMap into two parts: deterministic (`det_part`) and non-deterministic (`non_det_part`). `det_part` is used while we sure that allocation and insertion order of def ids is deterministic, at the moment of writing it happens before we start parallel checks after prefetch of `hir_crate_items` in `run_required_analysis`. Up until this point of compilation the allocation of def ids and their insertion order into `det_part` should be the same between different compilations.

Next, when non-determinism starts due to parallel compilation we put all mapping between local hash and def ids into a `SortedMap` which gives us ready-to-use sorted by local hash slice of pairs to encode while encoding metadata.

Note, that this PR does not solve the problem of allocation of different def indices to same code entities (meaning same local hash), this problem is solved in #162809 by remapping needed local def indices.
Also note that we serialize `det_part` as a raw bytes sequence, so during remapping if we do not place all def indices that are needed to be remapped into separate container we will end up with copying `det_part`, changing some entries in it and then serialize it as a bytes sequence. With the approach in this PR we do not copy and modify `det_part`, instead we do all modifications in `non_det_part`.

r? @petrochenkov
pull Bot pushed a commit to xtqqczze/rust-lang-miri that referenced this pull request Sep 29, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#163327 (remove `MutTy`)
 - rust-lang/rust#162910 (Deterministic encoding of `DefPathHashMap`)
 - rust-lang/rust#163009 (dont store arbitrary parsed attributes in thir)
 - rust-lang/rust#162683 (Use attribute parser for `#[inline()]` attribute check)
 - rust-lang/rust#163429 (lint on `Ident::from_str_and_span` taking a string literal)
 - rust-lang/rust#163431 (Allow `#[repr(simd)]` with `f16b`)
 - rust-lang/rust#163433 (Support also `try-jobs:` to specify custom try jobs)
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 1, 2026
…nathanBrouwer

Rollup of 7 pull requests

Successful merges:

 - rust-lang#163327 (remove `MutTy`)
 - rust-lang#162910 (Deterministic encoding of `DefPathHashMap`)
 - rust-lang#163009 (dont store arbitrary parsed attributes in thir)
 - rust-lang#162683 (Use attribute parser for `#[inline()]` attribute check)
 - rust-lang#163429 (lint on `Ident::from_str_and_span` taking a string literal)
 - rust-lang#163431 (Allow `#[repr(simd)]` with `f16b`)
 - rust-lang#163433 (Support also `try-jobs:` to specify custom try jobs)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants