Gather module items after lowering.

[cjgillot]

This avoids having a non-local analysis inside lowering.

By implementing hir_module_items using a visitor, we make sure that iterations and visitors are consistent.

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 5ffc563af96b1ff14400de3a9b370af50cc5754c with merge f7a3ac3869f210540060c13cd4a57d8752d354f1...

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit e6cb0fbcfe8a5e17f29cde60d6cf93e7302ef6a0 with merge f9be3c7a2baa589da11754c257a4cef83524c9f7...

[bors]

☀️ Try build successful - checks-actions
Build commit: f9be3c7a2baa589da11754c257a4cef83524c9f7 (f9be3c7a2baa589da11754c257a4cef83524c9f7)

[rust-timer]

Queued f9be3c7a2baa589da11754c257a4cef83524c9f7 with parent 8ceea01, future comparison URL.

[jackh726]

r? @petrochenkov

[rust-timer]

Finished benchmarking commit (f9be3c7a2baa589da11754c257a4cef83524c9f7): comparison url.

Summary: This change led to small relevant mixed results 🤷 in compiler performance.

• Small improvement in instruction counts (up to -0.5% on full builds of ctfe-stress-4)
• Very small regression in instruction counts (up to 1.1% on incr-patched: println builds of coercions)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[petrochenkov]

r? @petrochenkov

Nooo, I only left a comment...

[petrochenkov]

Looks like this PR does two different things:

• Moves the global analysis pass from lowering to somewhere else.
• Changes how iteration through modules is done - walking a single static list (no allocations or other overhead, maximum parallelism) vs. walking many lists of submodules (allocations in the queue, parallelism is apparently reduces as well).

The second part may be responsible for the perf differences.

[jackh726]

r? @petrochenkov

Nooo, I only left a comment...

oops, tag? (but for real, I can take back the review if you've got too much on your plate, just figured this was more along what you'd normally review ♥)

[petrochenkov]

@bors r+

[bors]

📌 Commit 91575f8 has been approved by petrochenkov

[bors]

⌛ Testing commit 91575f8 with merge 7b5f952...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 7b5f952 to master...

[rust-timer]

Finished benchmarking commit (7b5f952): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

• Moderate improvement in instruction counts (up to -1.3% on incr-unchanged builds of ctfe-stress-4)
• Moderate regression in instruction counts (up to 0.8% on incr-unchanged builds of tuple-stress)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

[pnkfelix]

Visited for weekly performance triage. Several benchmarks improved, by around -1% (+/-0.20%).

Some others regressed, by around +0.5% (+/- 0.2%)

To me, this seems like a wash; I'm not sure its worth spending time investigating the source of the regressions.

But since this was only labelled as a perf-regression two days ago, and my own analysis is based largely on skimming with my eyeballs, I'll hold off on officially triaging it; maybe someone will notice something I missed.