Skip to content

delegation: fix cycles during delayed lowering#154368

Open
aerooneqq wants to merge 15 commits intorust-lang:mainfrom
aerooneqq:delegation-force-lowering-later
Open

delegation: fix cycles during delayed lowering#154368
aerooneqq wants to merge 15 commits intorust-lang:mainfrom
aerooneqq:delegation-force-lowering-later

Conversation

@aerooneqq
Copy link
Copy Markdown
Contributor

@aerooneqq aerooneqq commented Mar 25, 2026

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

@rustbot 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 25, 2026
@aerooneqq aerooneqq changed the title delegation: invoke hir_crate_items before force_delayed_lowering delegation: invoke hir_crate_items before force_delayed_owners_lowering Mar 25, 2026
@petrochenkov petrochenkov added the F-fn_delegation `#![feature(fn_delegation)]` label Mar 25, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

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 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 Mar 25, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

Relevant - #154387, #154389.

@jyn514
Copy link
Copy Markdown
Member

jyn514 commented Mar 26, 2026

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

r? @Zoxc

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 26, 2026

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.

@rust-bors

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the delegation-force-lowering-later branch from f32739b to f50159f Compare March 27, 2026 09:41
@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@aerooneqq aerooneqq force-pushed the delegation-force-lowering-later branch from f50159f to 5afd6e4 Compare March 27, 2026 09:50
@aerooneqq
Copy link
Copy Markdown
Contributor Author

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.

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:

struct X<T> { t: T }

// No generics provided
impl X {
  pub fn foo() {}
}

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.

let item = tcx.hir_item(id);

@rustbot ready

@rustbot rustbot 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 Mar 27, 2026
@rust-log-analyzer

This comment has been minimized.

@aerooneqq aerooneqq changed the title delegation: invoke hir_crate_items before force_delayed_owners_lowering delegation: fix cycles during delayed lowering Mar 27, 2026
@petrochenkov 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 Mar 27, 2026
@aerooneqq
Copy link
Copy Markdown
Contributor Author

aerooneqq commented Mar 27, 2026

@rustbot ready
For perf.

@rustbot rustbot 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 Mar 27, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@petrochenkov
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 7, 2026
rust-bors bot pushed a commit that referenced this pull request Apr 7, 2026
…<try>

delegation: fix cycles during delayed lowering
@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 7, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 7, 2026

☀️ Try build successful (CI)
Build commit: 50b5f1c (50b5f1c585b99753d3ee178835cb2ab226e8fd04, parent: 49b6ac01d6f4c3da812039ae846407a20961aa4c)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (50b5f1c): comparison URL.

Overall result: ❌ regressions - 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.4% [0.2%, 0.8%] 90
Regressions ❌
(secondary)
0.7% [0.1%, 2.2%] 53
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [0.2%, 0.8%] 90

Max RSS (memory usage)

Results (primary 5.3%, secondary 2.9%)

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

mean range count
Regressions ❌
(primary)
5.6% [0.5%, 25.1%] 27
Regressions ❌
(secondary)
5.8% [0.9%, 19.1%] 13
Improvements ✅
(primary)
-2.4% [-2.4%, -2.4%] 1
Improvements ✅
(secondary)
-2.6% [-5.9%, -0.9%] 7
All ❌✅ (primary) 5.3% [-2.4%, 25.1%] 28

Cycles

Results (primary -2.2%, secondary 2.7%)

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.3% [2.2%, 4.9%] 9
Improvements ✅
(primary)
-2.2% [-2.2%, -2.2%] 1
Improvements ✅
(secondary)
-3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) -2.2% [-2.2%, -2.2%] 1

Binary size

Results (primary 0.1%, secondary 0.1%)

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

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.1%] 65
Regressions ❌
(secondary)
0.1% [0.0%, 0.1%] 24
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [0.0%, 0.1%] 65

Bootstrap: 487.434s -> 486.33s (-0.23%)
Artifact size: 395.10 MiB -> 395.16 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 7, 2026
@aerooneqq
Copy link
Copy Markdown
Contributor Author

@rustbot ready
Removed fix related to hir_opt_ident, can retry perf.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Apr 8, 2026
…<try>

delegation: fix cycles during delayed lowering
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 8, 2026
@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 8, 2026

☀️ Try build successful (CI)
Build commit: 764186e (764186e460a9233c2345f4defb35b8e4332bdf1d, parent: 30d0309fa821f7a0984a9629e0d227ca3c0d2eda)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (764186e): comparison URL.

Overall result: ❌ regressions - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 7.5%, secondary 6.9%)

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

mean range count
Regressions ❌
(primary)
7.5% [3.0%, 24.8%] 19
Regressions ❌
(secondary)
6.9% [1.3%, 19.8%] 11
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 7.5% [3.0%, 24.8%] 19

Cycles

Results (secondary 13.3%)

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)
13.3% [13.2%, 13.5%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (primary 0.1%, secondary 0.1%)

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

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.1%] 46
Regressions ❌
(secondary)
0.1% [0.0%, 0.1%] 24
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [0.0%, 0.1%] 46

Bootstrap: 489.264s -> 503.193s (2.85%)
Artifact size: 395.30 MiB -> 395.32 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 8, 2026
@aerooneqq
Copy link
Copy Markdown
Contributor Author

Those 5 regressions happened because hir_crate was called during hir_module_items, now it should be ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

F-fn_delegation `#![feature(fn_delegation)]` perf-regression Performance regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delegation: oom

6 participants