Avoid loading HIR for check_well_formed on type declarations by oli-obk · Pull Request #143328 · rust-lang/rust

oli-obk · 2025-07-02T13:20:12Z

compiler-errors · 2025-07-02T16:52:11Z

Avoid loading HIR for check_well_formed on type declarations r? `@compiler-errors` the last commit is quite hacky. I could add a `ty_span` query to work together with `def_span` and `def_ident_span`, which would nicely solve all those cases, but potentially add lots of new dep graph edges

bors · 2025-07-02T16:53:22Z

⌛ Trying commit ae395ef with merge 51abd09...

bors · 2025-07-02T19:10:17Z

☀️ Try build successful - checks-actions
Build commit: 51abd09 (51abd0965b9c4c4221f1da2e6b99581d81b54955)

rust-timer · 2025-07-03T15:39:56Z

Finished benchmarking commit (51abd09): 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
Regressions ❌ (secondary)	0.5%	[0.4%, 1.0%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.1%]	11
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.4%, secondary 1.4%)

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

	mean	range	count
Regressions ❌ (primary)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	2.3%	[0.4%, 4.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.5%, -0.5%]	1
All ❌✅ (primary)	2.4%	[2.4%, 2.4%]	1

Cycles

Results (primary 0.7%, secondary 2.1%)

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

	mean	range	count
Regressions ❌ (primary)	2.4%	[1.5%, 3.5%]	5
Regressions ❌ (secondary)	2.1%	[2.0%, 2.3%]	2
Improvements ✅ (primary)	-2.2%	[-2.7%, -1.6%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[-2.7%, 3.5%]	8

Binary size

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

Bootstrap: 462.452s -> 462.663s (0.05%)
Artifact size: 372.20 MiB -> 372.26 MiB (0.02%)

oli-obk · 2025-07-09T08:21:23Z

@bors try @rust-timer queue

let's see what the new cache hit perf table column says about this PR

bors · 2025-07-09T08:22:36Z

⌛ Trying commit 74ee3af with merge 2fb9abc...

Avoid loading HIR for check_well_formed on type declarations r? `@compiler-errors` the last commit is quite hacky. I could add a `ty_span` query to work together with `def_span` and `def_ident_span`, which would nicely solve all those cases, but potentially add lots of new dep graph edges

bors · 2025-07-09T10:36:26Z

☀️ Try build successful - checks-actions
Build commit: 2fb9abc (2fb9abc6e5a5c9600cce4af74c0608e8ba4a507e)

rust-timer · 2025-07-09T12:06:59Z

Finished benchmarking commit (2fb9abc): 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.1%, 0.4%]	13
Regressions ❌ (secondary)	0.6%	[0.0%, 2.0%]	17
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.1%]	1
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.2%]	6
All ❌✅ (primary)	0.2%	[-0.1%, 0.4%]	14

Max RSS (memory usage)

Results (primary -0.0%, secondary -2.5%)

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

	mean	range	count
Regressions ❌ (primary)	4.0%	[2.2%, 5.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.0%	[-4.4%, -3.7%]	2
Improvements ✅ (secondary)	-2.5%	[-2.6%, -2.4%]	2
All ❌✅ (primary)	-0.0%	[-4.4%, 5.8%]	4

Cycles

Results (primary -2.8%, secondary 2.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)	2.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-2.8%	[-3.2%, -2.5%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.8%	[-3.2%, -2.5%]	2

Binary size

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

Bootstrap: 465.667s -> 463.45s (-0.48%)
Artifact size: 372.26 MiB -> 372.26 MiB (0.00%)

oli-obk · 2025-07-14T09:04:08Z

Only instructions regressed, wall time and cycles either improved or are noise (completely unrelated queries).

Curious that it did not reduce HIR accesses, so I'll need to investigate a bit more

compiler-errors · 2025-07-16T19:32:15Z

Perf improvements look not beneficial here. Thoughts?

@rustbot author

rustbot · 2025-07-16T19:32:19Z

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

cjgillot · 2025-08-03T20:43:02Z

compiler/rustc_hir_analysis/src/collect.rs

    tcx.ensure_ok().generics_of(def_id);
    tcx.ensure_ok().type_of(def_id);
    tcx.ensure_ok().predicates_of(def_id);
 }


Should we inline this? Or rename and use it more generally?

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

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -1025,18 +1021,13 @@ fn check_type_defn<'tcx>(
                    }


Should we check def_kind is DefKind::AnonConst | DefKind::InlineConst before fetching HIR here?

rustbot assigned compiler-errors Jul 2, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 2, 2025

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 2, 2025

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 3, 2025

oli-obk force-pushed the ty-decl-wf-check branch from ae395ef to e3d6d33 Compare July 7, 2025 08:33

oli-obk added 4 commits July 8, 2025 07:21

Rename some functions that are not part of lowering

695e297

Perform trait checks in the type based wf checker

06cc8b9

Assert that we do not load HIR for things we do not have HIR checks for

36b7de1

Check structs/enums/unions without loading their HIR

74ee3af

oli-obk force-pushed the ty-decl-wf-check branch from e3d6d33 to 74ee3af Compare July 9, 2025 08:20

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 9, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 9, 2025

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 16, 2025

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

cjgillot reviewed Aug 3, 2025

View reviewed changes

Uh oh!

Conversation

oli-obk commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

compiler-errors commented Jul 2, 2025

Uh oh!

This comment has been minimized.

bors commented Jul 2, 2025

Uh oh!

bors commented Jul 2, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 3, 2025

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

Uh oh!

oli-obk commented Jul 9, 2025

Uh oh!

This comment has been minimized.

bors commented Jul 9, 2025

Uh oh!

bors commented Jul 9, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 9, 2025

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

Uh oh!

oli-obk commented Jul 14, 2025

Uh oh!

compiler-errors commented Jul 16, 2025

Uh oh!

rustbot commented Jul 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

oli-obk commented Jul 2, 2025 •

edited

Loading