Skip to content

rustdoc: use ThinVec for cleaned generics#102928

Merged
bors merged 3 commits into
rust-lang:masterfrom
notriddle:notriddle/rustdoc-thin-vec
Nov 4, 2022
Merged

rustdoc: use ThinVec for cleaned generics#102928
bors merged 3 commits into
rust-lang:masterfrom
notriddle:notriddle/rustdoc-thin-vec

Conversation

@notriddle

@notriddle notriddle commented Oct 11, 2022

Copy link
Copy Markdown
Contributor

Depends on mozilla/thin-vec#38

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 11, 2022
@rustbot

rustbot commented Oct 11, 2022

Copy link
Copy Markdown
Collaborator

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

cc @camelid

@rust-highfive

Copy link
Copy Markdown
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @CraftSpider (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 11, 2022
@notriddle notriddle marked this pull request as draft October 11, 2022 17:13
@notriddle

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 11, 2022
@bors

bors commented Oct 11, 2022

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 5b312b169d8148e6f52067f1bf28d8fea611ae18 with merge 234a079207af4dc967931c934e36a0c65cdaab6e...

@bors

bors commented Oct 11, 2022

Copy link
Copy Markdown
Collaborator

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

@rust-timer

Copy link
Copy Markdown
Collaborator

Queued 234a079207af4dc967931c934e36a0c65cdaab6e with parent cde693c, future comparison URL.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (234a079207af4dc967931c934e36a0c65cdaab6e): 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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.3% [-0.3%, -0.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.3% [-0.3%, -0.3%] 1

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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.7% [3.7%, 3.7%] 1
Improvements ✅
(primary)
-1.3% [-1.6%, -1.2%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.3% [-1.6%, -1.2%] 3

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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.9% [3.0%, 4.6%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 11, 2022
@notriddle

Copy link
Copy Markdown
Contributor Author

As expected, the three max-RSS improvements are to doc tests, while the one max-RSS regression is an opt test (so it's probably spurious).

@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Oct 12, 2022
@rust-log-analyzer

This comment has been minimized.

@notriddle

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 12, 2022
@bors

bors commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 1265a4d7660f55c54849a04edef25e3523a8866f with merge 5a731b66e857fa59cdf13badedcf079f0b34e8be...

@nnethercote

Copy link
Copy Markdown
Contributor

Which types are shrunk by this change? We have a practice of using static assertions to check the size of often-instantiated types, e.g. here. It might be useful to do likewise here.

@bors

bors commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator

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

@rust-timer

Copy link
Copy Markdown
Collaborator

Queued 5a731b66e857fa59cdf13badedcf079f0b34e8be with parent 538f118, future comparison URL.

@notriddle

notriddle commented Oct 12, 2022

Copy link
Copy Markdown
Contributor Author

@nnethercote rustdoc::clean::types::Generics, which is included as as member of Function, Trait, TraitAlias, Struct, Union, Enum, Typedef, OpaqueTy, Impl, and ItemKind::TyAssocTypeItem. It was 48 bytes before this PR, and 16 bytes afterward.

I've added a line to track this.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (5a731b66e857fa59cdf13badedcf079f0b34e8be): 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.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.3% [-0.3%, -0.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.3% [-0.3%, -0.3%] 1

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.

mean1 range count2
Regressions ❌
(primary)
2.5% [2.0%, 3.0%] 2
Regressions ❌
(secondary)
2.7% [2.0%, 5.1%] 9
Improvements ✅
(primary)
-1.5% [-1.8%, -1.3%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [-1.8%, 3.0%] 4

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.

mean1 range count2
Regressions ❌
(primary)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-2.7%, -2.7%] 1
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 12, 2022
@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle marked this pull request as ready for review November 2, 2022 23:17
@GuillaumeGomez

Copy link
Copy Markdown
Member

Nice!

@bors r+ rollup=iffy

@bors

bors commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit eb29696 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors 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 Nov 3, 2022
@bors

bors commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit eb29696 with merge 254797dfc7d500900b5cb4531b8f4f55d895e2c1...

tcx.generics_of(assoc_item.def_id),
ty::GenericPredicates { parent: None, predicates },
);
// Move bounds that are (likely) directly attached to the associated type

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you replaced the comment I updated with its older version ^^' No worries, I'm gonna revert that as part of a bigger PR in the future.

@bors

bors commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

💥 Test timed out

@bors 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 Nov 3, 2022
@notriddle

Copy link
Copy Markdown
Contributor Author

@bors 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 Nov 3, 2022
@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

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

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

@bors

bors commented Nov 4, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit eb29696 with merge 15d7556...

@bors

bors commented Nov 4, 2022

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 15d7556 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 4, 2022
@bors bors merged commit 15d7556 into rust-lang:master Nov 4, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 4, 2022
@notriddle notriddle deleted the notriddle/rustdoc-thin-vec branch November 4, 2022 07:03
@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (15d7556): 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
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.2% [-2.8%, -1.7%] 6
All ❌✅ (primary) - - 0

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)
-1.4% [-1.8%, -1.1%] 3
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) -1.4% [-1.8%, -1.1%] 3

Cycles

This benchmark run did not return any relevant results for this metric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-rustdoc-json Area: Rustdoc JSON backend merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants