Stable hashing cleanups by nnethercote · Pull Request #152086 · rust-lang/rust

nnethercote · 2026-02-04T06:11:01Z

I took a close look at rustc_query_system::ich and found a bunch of things to improve.

r? @cjgillot

nnethercote · 2026-02-04T06:11:33Z

@cjgillot: I'm not 100% sure you're the best reviewer, please reassign if appropriate.

cc @Zalathar @Zoxc

compiler/rustc_query_system/src/ich/hcx.rs

compiler/rustc_query_system/src/ich/impls_syntax.rs

Zoxc · 2026-02-04T07:08:31Z

LGTM

Kobzol · 2026-02-04T07:57:34Z

@bors try @rust-timer queue

Just to be sure :)

Stable hashing cleanups

nnethercote · 2026-02-04T08:59:53Z

@bors try cancel

rust-bors · 2026-02-04T08:59:57Z

Try build cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/21663283139

nnethercote · 2026-02-04T09:00:50Z

@bors try @rust-timer queue

Stable hashing cleanups

rust-bors · 2026-02-04T11:11:36Z

☀️ Try build successful (CI)
Build commit: 540561d (540561de691be003ef1a9f9bf263df00e951622f, parent: 794495e2b4a1353cf7b66ffc55f0e755490af2cc)

rust-timer · 2026-02-04T11:52:26Z

Finished benchmarking commit (540561d): 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.3%	[0.1%, 0.5%]	56
Regressions ❌ (secondary)	0.3%	[0.0%, 0.7%]	43
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.2%, -0.0%]	3
All ❌✅ (primary)	0.3%	[0.1%, 0.5%]	56

Max RSS (memory usage)

Results (primary -3.2%, secondary -2.0%)

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)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

Cycles

Results (secondary 2.8%)

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)	4.7%	[2.0%, 9.4%]	10
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.3%	[-5.1%, -1.0%]	3
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 475.808s -> 473.718s (-0.44%)
Artifact size: 398.08 MiB -> 397.89 MiB (-0.05%)

nnethercote · 2026-02-04T23:01:24Z

Locally I get small perf improvements, of a similar magnitude to the regressions seen on CI :( Cachegrind diffs indicate it's something about span_hash_stable. I'll try a #[inline] and if that doesn't help I'll remove the span_hash_stable change.

@bors try @rust-timer queue

rust-bors · 2026-02-05T06:08:12Z

☀️ Try build successful (CI)
Build commit: fb757e0 (fb757e05575e66e7e8fdf984852c1fa9ec457db8, parent: db3e99bbab28c6ca778b13222becdea54533d908)

rust-timer · 2026-02-05T06:49:12Z

Finished benchmarking commit (fb757e0): comparison URL.

Overall result: ❌ regressions - no action needed

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.

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

Max RSS (memory usage)

Results (secondary -1.8%)

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
Improvements ✅ (secondary)	-1.8%	[-2.3%, -1.0%]	3
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 3.6%)

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

Binary size

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

Bootstrap: 472.94s -> 473.52s (0.12%)
Artifact size: 398.12 MiB -> 398.12 MiB (0.00%)

nnethercote · 2026-02-05T07:21:13Z

I have removed the commit that I think is the problem

That worked.

@bors r=cjgillot

rust-bors · 2026-02-05T07:21:16Z

📌 Commit f87bdac has been approved by cjgillot

It is now in the queue for this repository.

It has a single use. It doesn't need to be public. It doesn't use `self` and so doesn't need to be in the trait. And `IGNORED_ATTRIBUTES` can be moved within it.

It has a single `bool` field.

Calling `match` on a struct is a really weird thing to do. As the name suggests, it's an assert, so let's write it as one. Also clarify the comment a little.

It reads the `HashingControls::span` field, but there is also the `hashing_controls` method. No need to have two different ways of doing it.

It has a single impl, which does nothing, as you'd expect when hashing a type called `HashIgnoredAttrId`! So this commit just removes it, and `HashIgnoredAttrId::hash_stable` ends up doing nothing (with a comment) instead of calling the trait method to do nothing.

The new type makes the behaviour clearer: we start with the cache in an "unused" form and then instantiate it once it is actually used.

They both have a single use. Also adjust a couple of visibilities.

`HashStableContext` impls should be in `hcx.rs`, and `HashStable` impls should be in `impls_syntax.rs`. This commit moves a few that are in the wrong file.

It only has two uses. We can instead use `with_stable_hashing_context`, which has more than 30 uses.

I tried removing it to see what happened. Not a good idea!

rustbot · 2026-02-05T22:18:54Z

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.

nnethercote · 2026-02-05T22:19:15Z

I rebased.

@bors r=cjgillot

rust-bors · 2026-02-05T22:19:19Z

📌 Commit e35fd45 has been approved by cjgillot

It is now in the queue for this repository.

rust-log-analyzer · 2026-02-05T23:28:28Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20260123.484
Commit: 6bd6555ca37d84114959e1c76d2c01448ff61c5d
Build Date: 2026-01-23T19:41:17Z
Worker ID: {e62b92fe-3a87-4ef8-97cb-d3bcaf35b9fb}
Azure Region: westcentralus
##[endgroup]
##[group]Operating System
Ubuntu
24.04.3
LTS
---
REPOSITORY                                   TAG       IMAGE ID       CREATED       SIZE
ghcr.io/dependabot/dependabot-updater-core   latest    bcec0b4e062b   10 days ago   783MB
=> Removing docker images...
Deleted Images:
untagged: ghcr.io/dependabot/dependabot-updater-core:latest
untagged: ghcr.io/dependabot/dependabot-updater-core@sha256:b662be51f7b8ef7e2c8464428f14e49cb79c36aa9afb7ecb9221dfe0f507050c
deleted: sha256:bcec0b4e062b5ffe11cc1c2729558c0cd96621c0271ab5e97ff3a56e0c25045a
deleted: sha256:64e147d5e54d9be8b8aa322e511cda02296eda4b8b8d063c6a314833aca50e29
deleted: sha256:5cba409bb463f4e7fa1a19f695450170422582c1bc7c0e934d893b4e5f558bc6
deleted: sha256:cddc6ebd344b0111eaab170ead1dfda24acdfe865ed8a12599a34d338fa8e28b
deleted: sha256:2412c3f334d79134573cd45e657fb6cc0abd75bef3881458b0d498d936545c8d
---
tests/ui/drain_collect.fixed ... ok
tests/ui/double_parens.rs ... ok
tests/ui/duplicate_underscore_argument.rs ... ok
tests/ui/duplicated_attributes.rs ... ok
tests/ui/duration_suboptimal_units.rs ... ok
tests/ui/duration_suboptimal_units_days_weeks.rs ... ok
tests/ui/duration_subsec.rs ... ok
tests/ui/duration_suboptimal_units_days_weeks.fixed ... ok
tests/ui/duration_suboptimal_units.fixed ... ok
tests/ui/double_parens.fixed ... ok
tests/ui/duration_subsec.fixed ... ok
tests/ui/else_if_without_else.rs ... ok
tests/ui/elidable_lifetime_names.rs ... ok
tests/ui/eager_transmute.rs ... ok
---
..............................................     (146/146)

======== tests/rustdoc-gui/search-filter.goml ========

[ERROR] line 48: Error: The CSS selector "#search-tabs .count.loading" still exists: for command `wait-for-false: "#search-tabs .count.loading"`
    at <file:///checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/test_docs/index.html?search=test>

======== tests/rustdoc-gui/search-result-display.goml ========

[WARNING] line 39: Delta is 0 for "x", maybe try to use `compare-elements-position` instead?

rustbot assigned cjgillot Feb 4, 2026

Zalathar reviewed Feb 4, 2026

View reviewed changes

compiler/rustc_query_system/src/ich/hcx.rs Outdated Show resolved Hide resolved

Zalathar reviewed Feb 4, 2026

View reviewed changes

compiler/rustc_query_system/src/ich/impls_syntax.rs Show resolved Hide resolved