Remove more attributes from metadata #98450

lqd · 2022-06-24T10:31:54Z

A lot of the attributes that are currently stored in the metadata aren't used at all. The biggest metadata usage comes from the doc attributes currently but they are needed by rustdoc so we only removed the ones that cannot be used in downstream crates (doc comments on private items).

r? @ghost

lqd · 2022-06-24T10:32:16Z

@bors try @rust-timer queue

rust-timer · 2022-06-24T10:32:17Z

Awaiting bors try build completion.

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

bors · 2022-06-24T10:32:25Z

⌛ Trying commit 27b259ccc15c4aba9396c486a2980575d1b8e120 with merge be1cdcd82f77e6d9756195ddabd239691e85bfc1...

bors · 2022-06-24T12:00:15Z

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

rust-timer · 2022-06-24T12:00:16Z

Queued be1cdcd82f77e6d9756195ddabd239691e85bfc1 with parent d017d59, future comparison URL.

jyn514 · 2022-06-24T13:06:44Z

Unsure of what this would break in rustdoc

Maybe just inlining? Inlining is where rustdoc takes items that are re-exported from one crate to another and shows the full documentation in the new crate (rather than a pub use xxx; item, which is the default across crates).

Unfortunately there's no way to predict that ahead of time because the inline annotation is in the downstream crate, not the dependency ...

rust-timer · 2022-06-24T13:17:56Z

Finished benchmarking commit (be1cdcd82f77e6d9756195ddabd239691e85bfc1): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-3.9%	-66.3%	64
Improvements 🎉 (secondary)	-7.5%	-19.5%	39
All 😿🎉 (primary)	-3.9%	-66.3%	64

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-3.7%	-14.1%	140
Improvements 🎉 (secondary)	-5.0%	-14.9%	108
All 😿🎉 (primary)	-3.7%	-14.1%	140

Cycles

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	2.7%	2.7%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-12.4%	-52.3%	18
Improvements 🎉 (secondary)	-14.7%	-20.9%	22
All 😿🎉 (primary)	-11.6%	-52.3%	19

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

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

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

lqd · 2022-06-24T13:38:43Z

Maybe just inlining? Inlining is where rustdoc takes items that are re-exported from one crate to another and shows the full documentation in the new crate (rather than a pub use xxx; item, which is the default across crates).

Yes, it seems some of the rustdoc inline-cross and intra-doc across crates tests fail indeed

lqd · 2022-06-24T13:45:11Z

Ignoring the doc benchmarks which I'm not even sure are correctly handled by this PR (even though I initially stumbled upon this by seeing all of the libcore doc comments being hashed in a check build of stm32f4-0.14.0), there seems to be some small wins in the regular profiles.

lqd · 2022-06-24T14:13:39Z

I was mostly interested in looking at metadata size, for libcore (-10%)

54M     libcore-d017d59ed013a4bc2431d023077eb7209fe9c60d.rmeta
49M     libcore-be1cdcd82f77e6d9756195ddabd239691e85bfc1.rmeta

and libstd (-15%)

8.7M    libstd-d017d59ed013a4bc2431d023077eb7209fe9c60d.rmeta
7.4M    libstd-be1cdcd82f77e6d9756195ddabd239691e85bfc1.rmeta

btw @jyn514 can I reproduce the rustdoc benchmarks simply by running cargo doc (optionally with flags defined in perf-config.json) ?

(if that's the case, then the only difference in the stm32f4-0.14.0 generated docs is in the search-index, which is 8% smaller. It's not straightforward to diff, it's very big for that benchmark, 3.6MB)

camelid · 2022-06-24T21:46:39Z

(if that's the case, then the only difference in the stm32f4-0.14.0 generated docs is in the search-index, which is 8% smaller. It's not straightforward to diff, it's very big for that benchmark, 3.6MB)

Perhaps the search-index is shrinking because rustdoc sees that there are no docs for the external items and is deciding not to inline them? Otherwise I can't think of why the search-index would shrink with this change.

lqd · 2022-09-05T12:51:46Z

@bors try @rust-timer queue

rust-timer · 2022-09-05T12:51:48Z

Awaiting bors try build completion.

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

bors · 2022-09-05T12:51:55Z

⌛ Trying commit 8da1bc8c2db72836f75891e8de593e03aa26fdfa with merge ec24c7b6173ddb76e1308214ea2b4bd1520ab4fb...

GuillaumeGomez · 2022-09-05T13:20:13Z

Here are the diff when we only remove unreachable items' documentation:

rmeta lib	Before	After	Diff
libaddr2line-92bc8313d5711253.rlib	497288	496488	0.2%
libadler-ce8b9a1966280505.rlib	64734	64734	0%
liballoc-5d73a6291bdcf84d.rlib	7186814	7162374	0.4%
libcfg_if-6ce0c15b4b6b1a04.rlib	9958	9958	0%
libcompiler_builtins-d130db2e7ffa5e06.rlib	1588808	1577096	0.7%
libcore-95c594ea0242ac01.rlib	61091692	60998780	0.2%
libgetopts-9b299cdaedbd7503.rlib	689262	688054	0.2%
libgimli-eec949a0a15fca68.rlib	7142216	7135440	0.1%
libhashbrown-f6d925553742e1b3.rlib	1606200	1603320	0.2%
liblibc-6cf37d22eaa1f096.rlib	3264616	3263624	0.3%
libLLVM-15-rust-1.65.0-nightly.so	109697528	109697528	0%
libmemchr-a8101b21344f81eb.rlib	1431468	1389316	3%
libminiz_oxide-44bf32af694d4f3a.rlib	1001630	992254	1%
libobject-af2efcb345913075.rlib	8630976	8630408	0%
libpanic_abort-6da9002fe89b2e2a.rlib	11180	11180	0%
libpanic_unwind-c2d7eb851cafcedd.rlib	39498	37186	5.9%
libproc_macro-4b89ad880cf901ef.rlib	4199496	`4189040`	0.3%
librustc_demangle-e1681d60af5e6ee1.rlib	591966	586742	0.9%
librustc_std_workspace_alloc-091abb8a2f3ef39a.rlib	5180	5180	0%
librustc_std_workspace_core-6f37b8376277b571.rlib	6722	6722	0%
librustc_std_workspace_std-879df989f3ba3bc5.rlib	8384	8384	0%
libstd-b6dbfa4842ad1467.rlib	15548624	15324024	1.5%
libstd-b6dbfa4842ad1467.so*	`7205640`	7071784	2%
libstd_detect-4062402ee65c0247.rlib	507760	504168	0.7%
libtest-c7b87c602ca2d4e9.rlib	4840816	4835280	0.1%
libtest-c7b87c602ca2d4e9.so*	1491912	1488984	0.2%
libunicode_width-b08619fcd1265a63.rlib	162844	162844	0%
libunwind-58a66e84df914c1c.rlib	46256	46256	0%

An idea we discussed with @lqd was to split the doc comments into their own metadata file (so we would have .rlib and .rdoc or something like that) which would allow to greatly benefit "normal" compilation. To be discussed I guess.

bors · 2022-09-05T14:42:13Z

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

rust-timer · 2022-09-05T14:42:15Z

Queued ec24c7b6173ddb76e1308214ea2b4bd1520ab4fb with parent 6e4a9ab, future comparison URL.

rust-timer · 2022-09-06T01:51:34Z

Finished benchmarking commit (ec24c7b6173ddb76e1308214ea2b4bd1520ab4fb): comparison URL.

Overall result: ❌ regressions - 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.

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

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

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)	1.2%	[0.5%, 1.8%]	6
Regressions ❌ (secondary)	4.6%	[2.4%, 8.4%]	3
Improvements ✅ (primary)	-1.3%	[-1.3%, -1.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[-1.3%, 1.8%]	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)	2.9%	[2.9%, 2.9%]	1
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-2.7%, -2.7%]	1
All ❌✅ (primary)	2.9%	[2.9%, 2.9%]	1

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

bors · 2022-10-21T01:25:58Z

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

rust-timer · 2022-10-21T01:26:00Z

Queued 9f7d8a6c2c3311e9e7231f533709680c98854e8b with parent 5ffa67d, future comparison URL.

rust-timer · 2022-10-21T03:56:08Z

Finished benchmarking commit (9f7d8a6c2c3311e9e7231f533709680c98854e8b): 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-review -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.7%	[0.4%, 0.9%]	2
Improvements ✅ (primary)	-2.7%	[-8.2%, -0.2%]	18
Improvements ✅ (secondary)	-5.5%	[-8.3%, -0.8%]	23
All ❌✅ (primary)	-2.7%	[-8.2%, -0.2%]	18

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)	-	-	0
Improvements ✅ (primary)	-2.4%	[-6.4%, -0.7%]	58
Improvements ✅ (secondary)	-5.5%	[-6.8%, -1.5%]	24
All ❌✅ (primary)	-2.4%	[-6.4%, -0.7%]	58

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)	-3.3%	[-6.7%, -2.0%]	9
Improvements ✅ (secondary)	-5.3%	[-7.0%, -2.9%]	18
All ❌✅ (primary)	-3.3%	[-6.7%, -2.0%]	9

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

GuillaumeGomez · 2022-10-21T12:21:25Z

After discussion with @lqd, seems like we're ready here. We will very likely follow-up this up with a RFC about storing attributes in another separate file to reduce this even further.

@bors: r=lqd,GuillaumeGomez

bors · 2022-10-21T12:21:26Z

📌 Commit 41263d2 has been approved by lqd,GuillaumeGomez

It is now in the queue for this repository.

bors · 2022-10-21T15:27:59Z

⌛ Testing commit 41263d2 with merge ba9d01b...

bors · 2022-10-21T18:08:53Z

☀️ Test successful - checks-actions
Approved by: lqd,GuillaumeGomez
Pushing ba9d01b to master...

rust-timer · 2022-10-21T20:42:35Z

Finished benchmarking commit (ba9d01b): 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
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-2.7%	[-8.2%, -0.2%]	18
Improvements ✅ (secondary)	-5.5%	[-8.4%, -0.8%]	23
All ❌✅ (primary)	-2.7%	[-8.2%, -0.2%]	18

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)	-	-	0
Improvements ✅ (primary)	-3.5%	[-7.0%, -0.6%]	30
Improvements ✅ (secondary)	-5.9%	[-7.3%, -2.5%]	23
All ❌✅ (primary)	-3.5%	[-7.0%, -0.6%]	30

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)	-3.6%	[-7.2%, -2.0%]	11
Improvements ✅ (secondary)	-5.8%	[-7.5%, -3.2%]	19
All ❌✅ (primary)	-3.6%	[-7.2%, -2.0%]	11

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

rustc_metadata: Encode even less doc comments The fact that `def_id` is in the `tcx.privacy_access_levels(())` table is not very meaningful, especially after rust-lang#102026, `is_exported` (or `is_reachable` in the worst case) is what you need. Follow up to rust-lang#98450. r? `@GuillaumeGomez` `@lqd`

rustc_metadata: Encode even less doc comments The fact that `def_id` is in the `tcx.privacy_access_levels(())` table is not very meaningful, especially after rust-lang/rust#102026, `is_exported` (or `is_reachable` in the worst case) is what you need. Follow up to rust-lang/rust#98450. r? `@GuillaumeGomez` `@lqd`

Add documentation for TyCtxt::visibility We encountered this issue while working on rust-lang/rust#98450. cc ``@lqd`` r? ``@cjgillot``

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 24, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 24, 2022

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 Jun 24, 2022

Mark-Simulacrum 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 Jul 24, 2022

GuillaumeGomez force-pushed the doc-metadata branch from 27b259c to 8da1bc8 Compare September 5, 2022 12:07

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 5, 2022

rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2022

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Oct 21, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 21, 2022

bors merged commit ba9d01b into rust-lang:master Oct 21, 2022

rustbot added this to the 1.66.0 milestone Oct 21, 2022

lqd deleted the doc-metadata branch October 21, 2022 20:18

petrochenkov mentioned this pull request Oct 24, 2022

rustc_metadata: Encode even less doc comments #103496

Merged

petrochenkov mentioned this pull request Jan 22, 2023

rustc_metadata: Fix encode_attrs #107171

Merged

WaffleLapkin mentioned this pull request Mar 2, 2023

Forbid the use of #[target_feature] on main #108651

Merged

GuillaumeGomez mentioned this pull request Apr 27, 2023

Rustdoc doesn't show repr for a struct re-exported from another package #110698

Closed

GuillaumeGomez mentioned this pull request Jan 17, 2024

Keep stable attr in metadata #120061

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove more attributes from metadata #98450

Remove more attributes from metadata #98450

lqd commented Jun 24, 2022 •

edited by GuillaumeGomez

Loading

lqd commented Jun 24, 2022

rust-timer commented Jun 24, 2022

bors commented Jun 24, 2022

bors commented Jun 24, 2022

rust-timer commented Jun 24, 2022

jyn514 commented Jun 24, 2022

rust-timer commented Jun 24, 2022

lqd commented Jun 24, 2022 •

edited

Loading

lqd commented Jun 24, 2022

lqd commented Jun 24, 2022 •

edited

Loading

camelid commented Jun 24, 2022

lqd commented Sep 5, 2022

rust-timer commented Sep 5, 2022

bors commented Sep 5, 2022

GuillaumeGomez commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 5, 2022

rust-timer commented Sep 6, 2022

bors commented Oct 21, 2022

rust-timer commented Oct 21, 2022

rust-timer commented Oct 21, 2022

GuillaumeGomez commented Oct 21, 2022

bors commented Oct 21, 2022

bors commented Oct 21, 2022

bors commented Oct 21, 2022

rust-timer commented Oct 21, 2022

Remove more attributes from metadata #98450

Remove more attributes from metadata #98450

Conversation

lqd commented Jun 24, 2022 • edited by GuillaumeGomez Loading

lqd commented Jun 24, 2022

rust-timer commented Jun 24, 2022

bors commented Jun 24, 2022

bors commented Jun 24, 2022

rust-timer commented Jun 24, 2022

jyn514 commented Jun 24, 2022

rust-timer commented Jun 24, 2022

Footnotes

lqd commented Jun 24, 2022 • edited Loading

lqd commented Jun 24, 2022

lqd commented Jun 24, 2022 • edited Loading

camelid commented Jun 24, 2022

lqd commented Sep 5, 2022

rust-timer commented Sep 5, 2022

bors commented Sep 5, 2022

GuillaumeGomez commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 5, 2022

rust-timer commented Sep 6, 2022

Overall result: ❌ regressions - ACTION NEEDED

Footnotes

bors commented Oct 21, 2022

rust-timer commented Oct 21, 2022

rust-timer commented Oct 21, 2022

Overall result: ✅ improvements - no action needed

Footnotes

GuillaumeGomez commented Oct 21, 2022

bors commented Oct 21, 2022

bors commented Oct 21, 2022

bors commented Oct 21, 2022

rust-timer commented Oct 21, 2022

Overall result: ✅ improvements - no action needed

Footnotes

lqd commented Jun 24, 2022 •

edited by GuillaumeGomez

Loading

lqd commented Jun 24, 2022 •

edited

Loading

lqd commented Jun 24, 2022 •

edited

Loading