Skip to content

Use interior mutability for StableHashingContext::caching_source_map.#154735

Closed
nnethercote wants to merge 3 commits intorust-lang:mainfrom
nnethercote:ref-Hcx
Closed

Use interior mutability for StableHashingContext::caching_source_map.#154735
nnethercote wants to merge 3 commits intorust-lang:mainfrom
nnethercote:ref-Hcx

Conversation

@nnethercote
Copy link
Copy Markdown
Contributor

@nnethercote nnethercote commented Apr 2, 2026 •
edited
Loading

This lets a lot of &mut hcx parameters change to &hcx, for symmetry with to_hash_stable_key, and avoids some unnecessary cloning of StableHashingContext. Details in individual commits.

@nnethercote
Use interior mutability for StableHashingContext::caching_source_map.
516ac2a 
`StableHashingContext::caching_source_map` is currently externally
mutated by `span_hash_stable`, which is reachable from many
`hash_stable` methods. But the mutation that happens is just some
internal caching, via `CachingSourceMap`. Conceptually it feels like a
read-only lookup of source map data.

This commit changes the field to interior mutability. This will let
`hash_stable` use `&Hcx` instead of `&mut Hcx`. The switch involves
replacing the existing `StableHashingContext::source_map` method with a
new `StableHashingContxt::span_data_to_lines_and_cols` method, because
returning a `&mut` was problematic with the use of `RefCell`. The
existing `CachingSourceMapView::span_data_to_lines_and_cols` now returns
an `Arc`, but that seems fine because the nearby
`CachingSourceMapView::byte_pos_to_line_and_col` already does that.
@nnethercote
Pass &hcx instead of &mut Hcx to hash_stable.
bd9b5ae 
This was enabled by the previous commit. A few related functions also
get a similar change.
@nnethercote
Avoid cloning of StableHashingContext.
79b99e6 
It's no longer needed now that `hash_stable` takes a `&hcx` instead of
`&mut hcx`.
@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 Apr 2, 2026
@nnethercote
Copy link
Copy Markdown
Contributor Author

nnethercote commented Apr 2, 2026

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
rust-bors bot pushed a commit that referenced this pull request Apr 2, 2026
@bors
Auto merge of #154735 - nnethercote:ref-Hcx, r=<try>
f43d2c0 
Use interior mutability for `StableHashingContext::caching_source_map`.
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 2, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 3, 2026

☔ The latest upstream changes (presumably #154727) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 3, 2026

☀️ Try build successful (CI)
Build commit: f43d2c0 (f43d2c0d111676f3fea7786460af17e466d1455b, parent: e6b64a2f4c696b840f8a384ec28690eed6a5d267)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Copy Markdown
Collaborator

rust-timer commented Apr 3, 2026

Finished benchmarking commit (f43d2c0): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@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.2% [0.2%, 0.3%] 10
Regressions ❌
(secondary)		 0.2% [0.1%, 0.3%] 23
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.1% [-0.1%, -0.0%] 3
All ❌✅ (primary) 0.2% [0.2%, 0.3%] 10

Max RSS (memory usage)

Results (primary -2.2%, secondary 0.4%)

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.4% [2.2%, 4.6%] 2
Improvements ✅
(primary)		 -2.2% [-2.2%, -2.2%] 1
Improvements ✅
(secondary)		 -2.7% [-3.6%, -1.8%] 2
All ❌✅ (primary) -2.2% [-2.2%, -2.2%] 1

Cycles

Results (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)		 - - 0
Regressions ❌
(secondary)		 6.3% [3.9%, 8.5%] 3
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -4.5% [-4.8%, -4.2%] 3
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 487.024s -> 488.913s (0.39%)
Artifact size: 395.12 MiB -> 395.23 MiB (0.03%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Apr 3, 2026
@nnethercote nnethercote mentioned this pull request Apr 3, 2026
Open
@nnethercote
Copy link
Copy Markdown
Contributor Author

nnethercote commented Apr 3, 2026

Slight performance regression. #154744 is an alternative that is perf-neutral.

@nnethercote nnethercote closed this Apr 3, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) perf-regression Performance regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nnethercote @rust-timer @rustbot