rustc_metadata: Filter encoded data more aggressively using `DefKind` #109765

petrochenkov · 2023-03-30T10:49:05Z

I focused on data that contains spans, because spans are expensive to encode/decode/hash, but also touched should_encode_visibility too.

One incorrect use of impl visibility in diagnostics is also replaced with trait visibility.

rustbot · 2023-03-30T10:49:11Z

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

petrochenkov · 2023-03-30T10:49:37Z

@bors try @rust-timer queue

bors · 2023-03-30T10:49:46Z

⌛ Trying commit 462c49f2301dd11b5634a8f07beb58732bbf3f9b with merge bef788b9e938a86104d5e6fd1c413440c5d305a2...

rustbot · 2023-03-30T13:23:11Z

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

petrochenkov · 2023-03-30T13:23:38Z

@bors try @rust-timer queue

bors · 2023-03-30T13:23:47Z

⌛ Trying commit 450109fe5dbfb024ad330bcfdd3f8741d0b51700 with merge c106784be1adfcb22d4ef5333f61698de37db28e...

bors · 2023-03-30T15:23:41Z

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

rust-timer · 2023-03-30T18:05:37Z

Finished benchmarking commit (c106784be1adfcb22d4ef5333f61698de37db28e): comparison URL.

Overall result: ✅ improvements - no action needed

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 may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.7%, -0.2%]	100
Improvements ✅ (secondary)	-1.1%	[-3.4%, -0.1%]	37
All ❌✅ (primary)	-0.9%	[-1.7%, -0.2%]	100

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[2.8%, 3.2%]	3
Improvements ✅ (primary)	-1.3%	[-2.0%, -0.5%]	7
Improvements ✅ (secondary)	-1.7%	[-2.2%, -0.9%]	4
All ❌✅ (primary)	-1.3%	[-2.0%, -0.5%]	7

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.9%	[-2.9%, -0.8%]	35
Improvements ✅ (secondary)	-3.1%	[-5.0%, -1.3%]	14
All ❌✅ (primary)	-1.9%	[-2.9%, -0.8%]	35

petrochenkov · 2023-03-30T19:09:45Z

@rustbot ready

petrochenkov · 2023-03-30T19:12:07Z

Binary size improvements are also nice - https://perf.rust-lang.org/compare.html?start=8a7ca936e61d04399198911ee2b07ac110bf17b0&end=c106784be1adfcb22d4ef5333f61698de37db28e&stat=size%3Alinked_artifact.

cjgillot

Those large matches are not the easiest thing to review...
How did you determine them?
Is there any way to be certain, or should we just wait for bug reports?

cjgillot · 2023-03-30T20:26:32Z

tests/ui/associated-types/associated-types-in-ambiguous-context.stderr

@@ -21,10 +21,17 @@ error[E0223]: ambiguous associated type
 LL | type X = std::ops::Deref::Target;
   |          ^^^^^^^^^^^^^^^^^^^^^^^
   |
-help: if there were a type named `Example` that implemented `Deref`, you could use the fully-qualified path
+help: use the fully-qualified path


Why does the output change?

Due to the compiler/rustc_hir_analysis/src/astconv/mod.rs change.
Previously, impl visibility was used, but impl visibility is meaningless (and no longer encoded with this PR), trait visibility was actually intended there.

cjgillot · 2023-03-30T20:27:31Z

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -825,25 +825,136 @@ fn should_encode_visibility(def_kind: DefKind) -> bool {
        | DefKind::ForeignTy
        | DefKind::TraitAlias
        | DefKind::AssocTy
+        | DefKind::TyParam


Why only TyParam and not LifetimeParam and ConstParam?

Some spans are only encoded for diagnostics.
Errors related to type parameters may report different diagnostics than errors related to other generic parameters.

It could make sense to drop extern spans for type parameter diagnostics too, to optimize the "good path".

petrochenkov · 2023-03-30T22:22:18Z

@cjgillot

How did you determine them?

Some common sense initially (named items that get into non-reexport module_items typically need to have everything encoded, others not so much), then ICEs from the test suite to fix non-obvious cases.

Is there any way to be certain, or should we just wait for bug reports?

Wait for bug reports, I guess.
If some rare case that is not currently covered by the test suite uses some missing piece of data, we can add it to the test suite and either encode that data or stop using it.

jackh726 · 2023-04-09T15:27:54Z

r? @cjgillot

petrochenkov · 2023-04-10T13:35:59Z

@bors r=cjgillot

bors · 2023-04-10T13:36:01Z

📌 Commit f5a9f6f has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-04-10T19:11:41Z

⌛ Testing commit f5a9f6f with merge 8b0362a3b018c68c3bd176a162565249a30eb272...

bors · 2023-04-10T19:13:27Z

💔 Test failed - checks-actions

rust-log-analyzer · 2023-04-11T02:33:38Z

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

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

petrochenkov · 2023-04-11T07:01:12Z

@bors retry

bors · 2023-04-11T10:32:44Z

⌛ Testing commit f5a9f6f with merge dfe024e...

bors · 2023-04-11T13:01:50Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing dfe024e to master...

rust-timer · 2023-04-11T14:17:29Z

Finished benchmarking commit (dfe024e): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.0%	[-1.8%, -0.2%]	101
Improvements ✅ (secondary)	-1.2%	[-3.6%, -0.1%]	34
All ❌✅ (primary)	-0.9%	[-1.8%, 0.7%]	102

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-1.5%	[-4.4%, -0.4%]	16
Improvements ✅ (secondary)	-1.0%	[-1.5%, -0.6%]	3
All ❌✅ (primary)	-1.5%	[-4.4%, -0.4%]	16

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.1%, 2.6%]	4
Improvements ✅ (primary)	-1.7%	[-2.7%, -0.6%]	38
Improvements ✅ (secondary)	-3.0%	[-4.2%, -2.3%]	12
All ❌✅ (primary)	-1.7%	[-2.7%, -0.6%]	38

rustbot assigned jackh726 Mar 30, 2023

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. labels Mar 30, 2023

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 Mar 30, 2023

This comment was marked as resolved.

Sign in to view

bors 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 30, 2023

petrochenkov force-pushed the encodeless branch from 462c49f to 450109f Compare March 30, 2023 13:23

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 30, 2023

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 30, 2023

cjgillot reviewed Mar 30, 2023

View reviewed changes

rustbot assigned cjgillot Apr 9, 2023

This comment was marked as resolved.

Sign in to view

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 Apr 10, 2023

petrochenkov added 2 commits April 10, 2023 15:47

rustc_metadata: Cleanup fn encode_info_for_item

9dd27b3

rustc_metadata: Filter encoded data more aggressively using DefKind

f5a9f6f

petrochenkov force-pushed the encodeless branch from 450109f to f5a9f6f Compare April 10, 2023 13:32

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 Apr 10, 2023

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 Apr 10, 2023

This comment has been minimized.

Sign in to view

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 Apr 11, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 11, 2023

bors merged commit dfe024e into rust-lang:master Apr 11, 2023

rustbot added this to the 1.70.0 milestone Apr 11, 2023

petrochenkov mentioned this pull request Apr 11, 2023

ICE: DefId does not have a "def_span" #110206

Closed

cjgillot mentioned this pull request Apr 17, 2023

Encode def span for ConstParam #110425

Merged

oli-obk mentioned this pull request Apr 21, 2023

Encode lifetime param spans too #110469

Merged

oli-obk mentioned this pull request May 9, 2023

Upgrade the toolchain to nightly-2023-04-16 model-checking/kani#2406

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_metadata: Filter encoded data more aggressively using `DefKind` #109765

rustc_metadata: Filter encoded data more aggressively using `DefKind` #109765

petrochenkov commented Mar 30, 2023

rustbot commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

This comment has been minimized.

bors commented Mar 30, 2023

This comment was marked as resolved.

rustbot commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

This comment has been minimized.

bors commented Mar 30, 2023

bors commented Mar 30, 2023

This comment has been minimized.

rust-timer commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

cjgillot left a comment

cjgillot Mar 30, 2023

petrochenkov Mar 30, 2023

cjgillot Mar 30, 2023

petrochenkov Mar 30, 2023

petrochenkov commented Mar 30, 2023

jackh726 commented Apr 9, 2023

This comment was marked as resolved.

petrochenkov commented Apr 10, 2023

bors commented Apr 10, 2023

bors commented Apr 10, 2023

bors commented Apr 10, 2023

This comment has been minimized.

rust-log-analyzer commented Apr 11, 2023

petrochenkov commented Apr 11, 2023

bors commented Apr 11, 2023

bors commented Apr 11, 2023

rust-timer commented Apr 11, 2023

rustc_metadata: Filter encoded data more aggressively using DefKind #109765

rustc_metadata: Filter encoded data more aggressively using DefKind #109765

Conversation

petrochenkov commented Mar 30, 2023

rustbot commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

This comment has been minimized.

bors commented Mar 30, 2023

This comment was marked as resolved.

rustbot commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

This comment has been minimized.

bors commented Mar 30, 2023

bors commented Mar 30, 2023

This comment has been minimized.

rust-timer commented Mar 30, 2023

Overall result: ✅ improvements - no action needed

petrochenkov commented Mar 30, 2023

petrochenkov commented Mar 30, 2023

cjgillot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrochenkov commented Mar 30, 2023

jackh726 commented Apr 9, 2023

This comment was marked as resolved.

petrochenkov commented Apr 10, 2023

bors commented Apr 10, 2023

bors commented Apr 10, 2023

bors commented Apr 10, 2023

This comment has been minimized.

rust-log-analyzer commented Apr 11, 2023

petrochenkov commented Apr 11, 2023

bors commented Apr 11, 2023

bors commented Apr 11, 2023

rust-timer commented Apr 11, 2023

Overall result: ✅ improvements - no action needed

rustc_metadata: Filter encoded data more aggressively using `DefKind` #109765

rustc_metadata: Filter encoded data more aggressively using `DefKind` #109765