Prehash visibilities in resolver by cjgillot · Pull Request #143371 · rust-lang/rust

cjgillot · 2025-07-03T13:55:29Z

Based on #143247
r? @ghost for perf

cjgillot · 2025-07-04T14:02:59Z

@bors try @rust-timer queue

Prehash visibilities in resolver Based on #143247 r? `@ghost` for perf

bors · 2025-07-04T14:04:10Z

⌛ Trying commit 11751ce with merge 556648c...

bors · 2025-07-04T16:19:30Z

☀️ Try build successful - checks-actions
Build commit: 556648c (556648c0fadc2403580e32dad35144435e964fcc)

rust-timer · 2025-07-04T18:47:36Z

Finished benchmarking commit (556648c): 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)	3.0%	[3.0%, 3.0%]	1
Regressions ❌ (secondary)	0.4%	[0.3%, 0.4%]	2
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[-0.3%, 3.0%]	2

Max RSS (memory usage)

Results (primary -1.9%, 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)	-1.9%	[-2.7%, -1.3%]	4
Improvements ✅ (secondary)	-2.0%	[-3.3%, -0.5%]	4
All ❌✅ (primary)	-1.9%	[-2.7%, -1.3%]	4

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)	1.0%	[1.0%, 1.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-3.0%, -0.6%]	2
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 1.1%)

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

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

Bootstrap: 461.146s -> 462.46s (0.28%)
Artifact size: 372.20 MiB -> 372.16 MiB (-0.01%)

petrochenkov · 2025-07-21T15:03:54Z

+ let visibilities_hash = {
+ let mut hasher = StableHasher::new();
+ let mut hcx = self.create_stable_hashing_context();
+ self.visibilities_for_hashing.hash_stable(&mut hcx, &mut hasher);


I thought about doing this operation in fn feed_visibility and keeping only the hash in Resolver instead of the whole visibilities_for_hashing table.
Is creating the hashing context a more or less expensive operation than adding a table element?

Creating a hashing context is very cheap. It's just moving some pointers around. I guess we could keep a StableHasher around, feed it the individual items, and finish into a hash here.

cjgillot · 2025-08-03T20:13:12Z

@bors try @rust-timer queue

rust-bors · 2025-08-03T20:13:16Z

⌛ Trying commit df4ffb9 with merge a723f79…

To cancel the try build, run the command @bors try cancel.

Prehash visibilities in resolver

rust-bors · 2025-08-03T22:28:52Z

☀️ Try build successful (CI)
Build commit: a723f79 (a723f792ac789ddf0784e86ea790d02add42793b, parent: f34ba774c78ea32b7c40598b8ad23e75cdac42a6)

rust-timer · 2025-08-04T00:34:33Z

Finished benchmarking commit (a723f79): comparison URL.

Overall result: ❌✅ regressions and improvements - 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
Regressions ❌ (secondary)	0.2%	[0.1%, 0.3%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	4
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.6%, 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)	1.3%	[1.1%, 1.5%]	2
Regressions ❌ (secondary)	3.8%	[2.3%, 5.3%]	8
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-1.9%	[-2.7%, -0.6%]	3
All ❌✅ (primary)	0.6%	[-0.8%, 1.5%]	3

Cycles

Results (primary -2.7%, 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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.6%]	2
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-2.7%	[-2.7%, -2.6%]	2

Binary size

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

Bootstrap: 467.629s -> 467.313s (-0.07%)
Artifact size: 376.97 MiB -> 376.93 MiB (-0.01%)

petrochenkov · 2025-08-06T10:39:11Z

@cjgillot
Could you answer #143371 (comment)?
r=me otherwise.
@rustbot author

rustbot · 2025-08-06T10:39:15Z

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

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 3, 2025

cjgillot force-pushed the prehash-visibilities branch from aaaefd1 to 17c75cbCompare July 3, 2025 14:06

petrochenkov self-assigned this Jul 4, 2025

petrochenkov added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 4, 2025

cjgillot force-pushed the prehash-visibilities branch from 17c75cb to 11751ceCompare July 4, 2025 14:01

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 Jul 4, 2025

bors added a commit that referenced this pull request Jul 4, 2025
Auto merge of#143371- cjgillot:prehash-visibilities, r=<try>
556648c
Prehash visibilities in resolver Based on #143247 r? `@ghost` for perf

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 Jul 4, 2025

cjgillot force-pushed the prehash-visibilities branch from 11751ce to 2a49c4cCompare July 20, 2025 13:26

petrochenkov 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 Jul 21, 2025

petrochenkov reviewed Jul 21, 2025
View reviewed changes

Comment threadcompiler/rustc_middle/src/hir/map.rs

petrochenkov marked this pull request as ready for review July 21, 2025 15:07

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 Jul 21, 2025

This comment was marked as resolved.
Sign in to view

Pre-hash visibilities in resolver.
df4ffb9

cjgillot force-pushed the prehash-visibilities branch from 2a49c4c to df4ffb9Compare August 3, 2025 20:13

This comment has been minimized.
Sign in to view

rust-borsBot added a commit that referenced this pull request Aug 3, 2025
Auto merge of#143371- cjgillot:prehash-visibilities, r=<try>
a723f79
Prehash visibilities in resolver

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 3, 2025

This comment has been minimized.
Sign in to view

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Aug 4, 2025

cjgillot 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 Aug 5, 2025

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 Aug 6, 2025

Uh oh!

Uh oh!

Conversation

cjgillot commented Jul 3, 2025

Uh oh!

cjgillot commented Jul 4, 2025

Uh oh!

This comment has been minimized.

bors commented Jul 4, 2025

Uh oh!

bors commented Jul 4, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 4, 2025

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

cjgillot commented Aug 3, 2025

Uh oh!

This comment has been minimized.

rust-borsBot commented Aug 3, 2025

Uh oh!

rust-borsBot commented Aug 3, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Aug 4, 2025

Overall result: ❌✅ regressions and improvements - no action needed

Uh oh!

petrochenkov commented Aug 6, 2025

Uh oh!

rustbot commented Aug 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants