delegation: fix cycles during delayed lowering

[aerooneqq]

View all comments

This PR forces lowering of delayed owners after hir_crate_items, as some diagnostics use hir_crate_items which results in query cycle which is then hangs calling def_path_str again and again. Fixes#154169. Part of #118212.

r? @petrochenkov

[petrochenkov]

What specific uses of def_path_str cause the infinite recursion?
Perhaps using something like with_reduced_queries will allow to break the recursion locally, and avoid global changes that this PR does.

[petrochenkov]

Relevant - #154387, #154389.

[jyn514]

This should really have a reviewer with a lot of query system experience.

r? @Zoxc

[rustbot]

Failed to set assignee to zoxc: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[aerooneqq]

So the reason of this cycle was that this function wants to iterate over all free items, however they were not available during delayed lowering as it was executed before hir_crate_items(()), now delayed lowering is executed after hir_crate_items(()), and we use available-without-lowering information to fill hir_crate_items with information about delayed owners.

rust/compiler/rustc_middle/src/ty/print/pretty.rs

Line 3393 in e3691a5

for id in tcx.hir_free_items(){

Next, there is one more problem in this function, which is maybe not that actual now, but it will be during supporting inherent impls, as during function resolution we will do coherence check, and in cases like:

structX<T>{t:T}// No generics providedimplX{pubfnfoo(){}}
reuse X::foo;

we would still come to for_each_def function, but this time this line will be a problem, as we would call delayed lowering during resolution of delegation, so I stored identifier which is needed by this loop in delayed owners, as it once again information that can be accessed without lowering.

rust/compiler/rustc_middle/src/ty/print/pretty.rs

Line 3398 in e3691a5

let item = tcx.hir_item(id);

@rustbot ready

[aerooneqq]

@rustbot ready
For perf.

[petrochenkov]

@bors try @rust-timer queue

[aerooneqq]

@rustbot ready

[petrochenkov]

@bors r+

[rust-bors]

📌 Commit dac2e3e has been approved by petrochenkov

It is now in the queue for this repository.

[rust-bors]

☀️ Test successful - CI
Approved by: petrochenkov
Duration: 3h 27m 53s
Pushing a87c9b9 to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 4c42051 (parent) -> a87c9b9 (this PR)

Test differences

Show 28 test diffs

Stage 1

• [ui] tests/ui/delegation/generics/query-cycle-oom-154169.rs: [missing] -> pass (J0)

Stage 2

• [ui] tests/ui/delegation/generics/query-cycle-oom-154169.rs: [missing] -> pass (J1)

Additionally, 26 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: x86_64-gnu-llvm-21-3, x86_64-gnu-llvm-22-3
• J1: aarch64-apple, aarch64-gnu, aarch64-gnu-llvm-21-1, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, optional-x86_64-gnu-parallel-frontend, test-various, x86_64-gnu, x86_64-gnu-debug, x86_64-gnu-gcc, x86_64-gnu-llvm-21, x86_64-gnu-llvm-21-2, x86_64-gnu-llvm-22-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard a87c9b96031d4d8698bb0cd6533e83bc6d77ddaa --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. dist-android: 22m 2s -> 28m 50s (+30.9%)
2. dist-x86_64-apple: 1h 43m -> 2h 10m (+26.5%)
3. i686-gnu-nopt-1: 1h 55m -> 2h 24m (+24.6%)
4. i686-gnu-2: 1h 26m -> 1h 42m (+18.0%)
5. armhf-gnu: 1h 18m -> 1h 32m (+17.4%)
6. pr-check-1: 27m 54s -> 32m 41s (+17.1%)
7. aarch64-gnu-llvm-21-1: 54m 22s -> 1h 3m (+16.7%)
8. i686-gnu-1: 2h 8m -> 2h 27m (+14.7%)
9. optional-x86_64-gnu-parallel-frontend: 2h 22m -> 2h 42m (+14.3%)
10. aarch64-gnu-debug: 1h 6m -> 1h 15m (+13.8%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (a87c9b9): comparison URL.

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

@rustbot label: -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.4%	[0.4%, 0.4%]	1
Regressions ❌ (secondary)	0.6%	[0.1%, 1.1%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.6%	[-3.2%, -0.1%]	2
All ❌✅ (primary)	0.4%	[0.4%, 0.4%]	1

Max RSS (memory usage)

Results (primary 6.5%, secondary 5.5%)

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

	mean	range	count
Regressions ❌ (primary)	7.0%	[0.6%, 25.1%]	20
Regressions ❌ (secondary)	5.5%	[1.0%, 17.4%]	12
Improvements ✅ (primary)	-2.8%	[-2.8%, -2.8%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	6.5%	[-2.8%, 25.1%]	21

Cycles

Results (primary 1.5%)

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

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

Binary size

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

Bootstrap: 489.441s -> 489.938s (0.10%)
Artifact size: 395.61 MiB -> 395.59 MiB (-0.00%)

[matthiaskrgr]

looks like doc memory regressed quite a bit
https://perf.rust-lang.org/compare.html?start=4c4205163abcbd08948b3efab796c543ba1ea687&end=a87c9b96031d4d8698bb0cd6533e83bc6d77ddaa&stat=max-rss