rustc_metadata: Optimize and document module children decoding #92086

petrochenkov · 2021-12-18T18:40:05Z

The first commit limits the item in the item_children/each_child_of_item query to modules (in name resolution sense) and adds a corresponding assertion.
The associated_item_def_ids query collecting children of traits and impls specifically now uses a simplified implementation not decoding unnecessary data instead of each_child_of_item, this gives a nice performance improvement.

The second commit does some renaming that clarifies the terminology used for all items in a module vs use items only.

rust-highfive · 2021-12-18T18:40:08Z

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

Some changes occurred in clean/types.rs.

cc @camelid

rust-highfive · 2021-12-18T18:40:09Z

r? @matthewjasper

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

petrochenkov · 2021-12-18T18:40:27Z

@bors try @rust-timer queue

rust-timer · 2021-12-18T18:40:28Z

Awaiting bors try build completion.

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

bors · 2021-12-18T18:40:35Z

⌛ Trying commit 62815a1f8ff0f1e06dd3f25b4aed88d32c2786f9 with merge 98c9bec5b9e4114d772623fd397e74d74b25da83...

bors · 2021-12-18T20:12:24Z

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

rust-timer · 2021-12-18T20:12:26Z

Queued 98c9bec5b9e4114d772623fd397e74d74b25da83 with parent d3848cb, future comparison URL.

rust-timer · 2021-12-19T01:30:16Z

Finished benchmarking commit (98c9bec5b9e4114d772623fd397e74d74b25da83): comparison url.

Summary: This change led to very large relevant improvements 🎉 in compiler performance.

Very large improvement in instruction counts (up to -14.5% on incr-unchanged builds of issue-88862)

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.

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

petrochenkov · 2021-12-21T05:58:50Z

This should be ready now.

petrochenkov · 2021-12-21T11:52:07Z

This is kind of a pre-requisite for #91795, so it's better to land it sooner.
r? rust-lang/compiler

jackh726

I think just one comment here.

For the most part looks okay to me, with the caveat that most of this code I've never really read before.

If it's not too much trouble, can you split the two major renames here to a separate commit? (item_children -> module_children and each_child_of_item -> get_module_children). That would make me more comfortable reviewing this,

compiler/rustc_metadata/src/rmeta/decoder.rs

jackh726 · 2021-12-22T09:01:02Z

compiler/rustc_metadata/src/rmeta/decoder.rs

+
+    /// Iterates over all named children of the given module,
+    /// including both proper items and reexports.
+    /// Module here is understood in name resolution sense - it can be a `mod` item,


The overloading of the term Module is definitely confusing....

…n(_untracked)` And `each_child_of_item` to `for_each_module_child`

Also rename `module_exports`/`export_map` to `module_reexports`/`reexport_map` for clarity.

petrochenkov · 2022-01-09T02:12:06Z

@bors r=jackh726

bors · 2022-01-09T02:12:08Z

📌 Commit 4b03fd9 has been approved by jackh726

bors · 2022-01-09T04:39:32Z

⌛ Testing commit 4b03fd9 with merge d4135a2a8b08d4b9afb1ae4980ed1b8914b6af14...

bors · 2022-01-09T08:54:32Z

💥 Test timed out

cjgillot · 2022-01-09T10:02:44Z

@bors retry

bors · 2022-01-09T10:02:52Z

⌛ Testing commit 4b03fd9 with merge e19ca1d...

rust-log-analyzer · 2022-01-09T10:08:57Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2022-01-09T12:43:31Z

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing e19ca1d to master...

rust-highfive · 2022-01-09T12:43:55Z

📣 Toolstate changed by #92086!

Tested on commit e19ca1d.
Direct link to PR: #92086

💔 miri on windows: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung).
💔 miri on linux: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung).

@eddyb

Tested on commit rust-lang/rust@e19ca1d. Direct link to PR: <rust-lang/rust#92086> 💔 miri on windows: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung). 💔 miri on linux: test-pass → build-fail (cc @eddyb @oli-obk @RalfJung).

rust-timer · 2022-01-09T15:02:32Z

Finished benchmarking commit (e19ca1d): comparison url.

Summary: This change led to very large relevant improvements 🎉 in compiler performance.

Very large improvement in instruction counts (up to -14.9% on incr-unchanged builds of issue-88862)

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

@rustbot label: -perf-regression

For us this mostly means changing `hir::exports::Export` to `metadata::ModChild` and `tcx.item_children` to `tcx.module_children`.

rust-highfive assigned matthewjasper Dec 18, 2021

rustbot added 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 Dec 18, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 18, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 18, 2021

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 18, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 19, 2021

petrochenkov force-pushed the modchild branch from 62815a1 to 53d308a Compare December 21, 2021 04:17

petrochenkov changed the title ~~[WIP] rustc_metadata: Optimize and document module children decoding~~ rustc_metadata: Optimize and document module children decoding Dec 21, 2021

rust-highfive assigned jackh726 and unassigned matthewjasper Dec 21, 2021

This comment has been minimized.

Sign in to view

jackh726 requested changes Dec 22, 2021

View reviewed changes

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 Dec 22, 2021

petrochenkov force-pushed the modchild branch 2 times, most recently from 2af260b to daade27 Compare December 23, 2021 08:41

petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 23, 2021

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2022

petrochenkov added 3 commits January 9, 2022 09:22

rustc_metadata: Optimize and document module children decoding

96c6a50

rustc_metadata: Rename item_children(_untracked) to `module_childre…

3051f6e

…n(_untracked)` And `each_child_of_item` to `for_each_module_child`

rustc_middle: Rename Export to ModChild and add some comments

4b03fd9

Also rename `module_exports`/`export_map` to `module_reexports`/`reexport_map` for clarity.

petrochenkov force-pushed the modchild branch from 0e18e03 to 4b03fd9 Compare January 9, 2022 02:11

bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 9, 2022

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

bors 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 Jan 9, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 9, 2022

bors merged commit e19ca1d into rust-lang:master Jan 9, 2022

rustbot added this to the 1.60.0 milestone Jan 9, 2022

rust-highfive mentioned this pull request Jan 9, 2022

miri no longer builds after rust-lang/rust#92086 #92691

Closed

This was referenced Jan 9, 2022

Fixes wrong unreachable_pub lints on nested and glob public reexport #87487

Merged

rustc_metadata: Stop passing CrateMetadataRef by reference (step 1) #92277

Merged

dario23 added a commit to dario23/rust-semverver that referenced this pull request May 4, 2022

Rustup to rust-lang/rust#92086

6ebd336

For us this mostly means changing `hir::exports::Export` to `metadata::ModChild` and `tcx.item_children` to `tcx.module_children`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_metadata: Optimize and document module children decoding #92086

rustc_metadata: Optimize and document module children decoding #92086

petrochenkov commented Dec 18, 2021 •

edited

Loading

rust-highfive commented Dec 18, 2021

rust-highfive commented Dec 18, 2021

petrochenkov commented Dec 18, 2021

rust-timer commented Dec 18, 2021

bors commented Dec 18, 2021

bors commented Dec 18, 2021

rust-timer commented Dec 18, 2021

rust-timer commented Dec 19, 2021

petrochenkov commented Dec 21, 2021

petrochenkov commented Dec 21, 2021

This comment has been minimized.

jackh726 left a comment

jackh726 Dec 22, 2021

petrochenkov commented Jan 9, 2022

bors commented Jan 9, 2022

bors commented Jan 9, 2022

bors commented Jan 9, 2022

cjgillot commented Jan 9, 2022

bors commented Jan 9, 2022

rust-log-analyzer commented Jan 9, 2022

bors commented Jan 9, 2022

rust-highfive commented Jan 9, 2022

rust-timer commented Jan 9, 2022

rustc_metadata: Optimize and document module children decoding #92086

rustc_metadata: Optimize and document module children decoding #92086

Conversation

petrochenkov commented Dec 18, 2021 • edited Loading

rust-highfive commented Dec 18, 2021

rust-highfive commented Dec 18, 2021

petrochenkov commented Dec 18, 2021

rust-timer commented Dec 18, 2021

bors commented Dec 18, 2021

bors commented Dec 18, 2021

rust-timer commented Dec 18, 2021

rust-timer commented Dec 19, 2021

petrochenkov commented Dec 21, 2021

petrochenkov commented Dec 21, 2021

This comment has been minimized.

jackh726 left a comment

Choose a reason for hiding this comment

jackh726 Dec 22, 2021

Choose a reason for hiding this comment

petrochenkov commented Jan 9, 2022

bors commented Jan 9, 2022

bors commented Jan 9, 2022

bors commented Jan 9, 2022

cjgillot commented Jan 9, 2022

bors commented Jan 9, 2022

rust-log-analyzer commented Jan 9, 2022

bors commented Jan 9, 2022

rust-highfive commented Jan 9, 2022

rust-timer commented Jan 9, 2022

petrochenkov commented Dec 18, 2021 •

edited

Loading