Merge BuildReducedGraphVisitor into DefPathVisitor by oli-obk · Pull Request #154945 · rust-lang/rust

oli-obk · 2026-04-07T11:16:26Z

These two visitors run right after each other on the same immutable AST. There's also a hash map for transferring the TyCtxtFeed created in the def collector to the BRG when it visits the same items. There are possibly more avenues for sharing logic, but I want to keep this PR simple.

only opening for perf runs for now. I'm still investigating how to ensure that future changes don't introduce subtle bugs by forgetting that def collection and reduced graph building are one pass now

Best reviewed commit-by-commit. I took a lot of care for making the individual changes reviewable, but all the Merge * commits aren't able to compile libcore until the last one.

oli-obk · 2026-04-07T11:16:39Z

@bors try @rust-timer queue

…try> Merge visitors def collector brg

rust-bors · 2026-04-07T13:26:45Z

☀️ Try build successful (CI)
Build commit: 5d54692 (5d5469299abfae0527f2b94b434252c5a94a80a3, parent: 49b6ac01d6f4c3da812039ae846407a20961aa4c)

rust-timer · 2026-04-07T14:07:08Z

Finished benchmarking commit (5d54692): comparison URL.

Overall result: ✅ 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.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.1%]	19
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.1%]	15
All ❌✅ (primary)	-0.3%	[-0.5%, -0.1%]	19

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)	1.9%	[1.4%, 2.3%]	3
Regressions ❌ (secondary)	3.0%	[1.0%, 4.5%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.0%	[-5.0%, -5.0%]	1
All ❌✅ (primary)	1.9%	[1.4%, 2.3%]	3

Cycles

Results (secondary -0.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)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.7%	[-3.7%, -3.7%]	1
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%]	63
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	24
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.1%]	63

Bootstrap: 487.434s -> 486.262s (-0.24%)
Artifact size: 395.10 MiB -> 395.19 MiB (0.02%)

…or::r`

oli-obk · 2026-04-20T12:05:21Z

@petrochenkov all further cleanups I can do will essentially be paired with the visibility feeding refactoring (not storing Feed anymore, but directly passing it to the brg logic or merging it further with the def collector where appropriate. I think complexity and effect wise it would be better to land this PR now and then do the follow ups in smaller versions, but I can also do everything here, or make it stacked PRs.

petrochenkov · 2026-04-20T15:37:33Z

@bors r+

rust-bors · 2026-04-20T15:37:36Z

📌 Commit f2c79f3 has been approved by petrochenkov

It is now in the queue for this repository.

rust-bors · 2026-04-21T02:16:15Z

☀️ Test successful - CI
Approved by: petrochenkov
Duration: 3h 6m 1s
Pushing 4fbae3e to main...

github-actions · 2026-04-21T02:19:02Z

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 66da6ca (parent) -> 4fbae3e (this PR)

Test differences

Show 1 test diff

1 doctest diff were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 4fbae3e30923ca23262b696d54ddbd176c2c38b4 --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

i686-msvc-2: 1h 45m -> 2h 18m (+31.4%)
dist-x86_64-illumos: 1h 25m -> 1h 44m (+22.6%)
i686-gnu-nopt-2: 2h 13m -> 1h 44m (-21.5%)
dist-aarch64-apple: 2h 19m -> 1h 51m (-20.1%)
pr-check-1: 34m 14s -> 27m 43s (-19.0%)
i686-msvc-1: 2h 57m -> 2h 23m (-19.0%)
x86_64-gnu-llvm-22-2: 1h 40m -> 1h 21m (-18.8%)
tidy: 3m 10s -> 2m 35s (-18.1%)
aarch64-apple: 3h 27m -> 2h 51m (-17.4%)
x86_64-rust-for-linux: 51m 33s -> 42m 49s (-16.9%)

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 · 2026-04-21T02:58:11Z

Finished benchmarking commit (4fbae3e): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.6%	[0.3%, 1.0%]	14
Improvements ✅ (primary)	-0.4%	[-0.5%, -0.2%]	6
Improvements ✅ (secondary)	-0.4%	[-1.1%, -0.2%]	13
All ❌✅ (primary)	-0.4%	[-0.5%, -0.2%]	6

Max RSS (memory usage)

Results (primary 1.4%, secondary 3.0%)

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

	mean	range	count
Regressions ❌ (primary)	1.4%	[1.4%, 1.4%]	1
Regressions ❌ (secondary)	3.0%	[1.3%, 5.1%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.4%	[1.4%, 1.4%]	1

Cycles

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

Binary size

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

Bootstrap: 496.86s -> 492.186s (-0.94%)
Artifact size: 394.38 MiB -> 394.40 MiB (0.01%)

Kobzol · 2026-04-21T17:45:20Z

The post-merge results are slightly worse than the pre-merge run, but overall it's a wash.

@rustbot label: +perf-regression-triaged

…=petrochenkov Immediately feed visibility on DefId creation This system was originally introduced in rust-lang#121089 This PR was enabled by refactorings in rust-lang#154945, because after that, the visibility feeding happens directly after the `DefId` creation, so we don't need to go through the intermediate hash table anymore Should unblock rust-lang#138995

Rollup merge of #155587 - oli-obk:feed-visibility-no-hash, r=petrochenkov Immediately feed visibility on DefId creation This system was originally introduced in #121089 This PR was enabled by refactorings in #154945, because after that, the visibility feeding happens directly after the `DefId` creation, so we don't need to go through the intermediate hash table anymore Should unblock #138995

…nkov Immediately feed visibility on DefId creation This system was originally introduced in rust-lang/rust#121089 This PR was enabled by refactorings in rust-lang/rust#154945, because after that, the visibility feeding happens directly after the `DefId` creation, so we don't need to go through the intermediate hash table anymore Should unblock rust-lang/rust#138995

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 7, 2026

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 Apr 7, 2026

rust-bors Bot pushed a commit that referenced this pull request Apr 7, 2026

Auto merge of #154945 - oli-obk:merge-visitors-def-collector-brg, r=<…

5d54692

…try> Merge visitors def collector brg

oli-obk changed the title ~~Merge visitors def collector brg~~ Merge BuildReducedGraphVisitor into DefPathVisitor Apr 7, 2026

This comment has been minimized.

Sign in to view

petrochenkov self-assigned this Apr 7, 2026

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 7, 2026

This comment has been minimized.

Sign in to view

oli-obk added 16 commits April 20, 2026 13:32

Rename DefCollector::resolver to r to match `BuildReducedGraphVisit…

8a4f0df

…or::r`

Merge BuildReducedGraphVisitor initialization into DefPathVisitor

393f3e0

Merge visit_item

90d0b41

Merge visit_foreign_item

a2cf11a

Merge visit_stmt

f98355e

Merge visit_block

3df9532

Merge visit_assoc_item

b1ec02f

Merge some simple visitor functions

05a49b6

Merge visit_generic_param

32ae2e5

Merge visit_where_predicate

51d1e83

Merge visit_crate

69aedb5

Merge visit_variant

d2f476e

Merge visit_field_def

f69d719

Merge visit_expr, visit_pat and visit_ty

d67a570

Merge visit_attribute

2892564

Fix use trees

0a714a2

oli-obk added 3 commits April 20, 2026 13:56

Inline macro calls in assoc item position logic

e617736

Avoid matching on the assoc item kind twice

c8862c7

Handle attributes entirely in the def collector

f2c79f3

oli-obk force-pushed the merge-visitors-def-collector-brg branch from 6fda0d7 to f2c79f3 Compare April 20, 2026 12:01

oli-obk marked this pull request as ready for review April 20, 2026 12:03

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 Apr 20, 2026

petrochenkov approved these changes Apr 20, 2026

View reviewed changes

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 20, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 21, 2026

rust-bors Bot merged commit 4fbae3e into rust-lang:main Apr 21, 2026
12 checks passed

rustbot added this to the 1.97.0 milestone Apr 21, 2026

This was referenced Apr 21, 2026

improve error message when a cfg-ed out item is resolved #154037

Open

Use per-parent disambiguators everywhere #155547

Merged

rustbot added the perf-regression Performance regression. label Apr 21, 2026

oli-obk mentioned this pull request Apr 21, 2026

Immediately feed visibility on DefId creation #155587

Merged

rustbot added the perf-regression-triaged The performance regression has been triaged. label Apr 21, 2026

Uh oh!

Conversation

oli-obk commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Apr 7, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Apr 7, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 7, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

This comment has been minimized.

oli-obk commented Apr 20, 2026

Uh oh!

petrochenkov commented Apr 20, 2026

Uh oh!

rust-bors Bot commented Apr 20, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Apr 21, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 21, 2026

Test differences

Job duration changes

Uh oh!

rust-timer commented Apr 21, 2026

Overall result: ❌✅ regressions and improvements - please read:

Uh oh!

Kobzol commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

oli-obk commented Apr 7, 2026 •

edited

Loading

Kobzol commented Apr 21, 2026 •

edited

Loading