Skip to content

Rustdoc: use ThinVec for GenericArgs bindings #92395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 3, 2022
Merged

Rustdoc: use ThinVec for GenericArgs bindings #92395

merged 1 commit into from
Jan 3, 2022

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Dec 29, 2021

The bindings are almost always empty. This reduces the size of PathSegment and GenericArgs by about one fourth.

@rust-highfive
Copy link
Contributor

Some changes occurred in clean/types.rs.

cc @camelid

@rustbot 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 29, 2021
@rust-highfive
Copy link
Contributor

r? @cjgillot

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

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

This comment has been minimized.

Sign in to view
@Kobzol Kobzol force-pushed the rustdoc-bindings-thin-vec branch from 86e1258 to 07c9b8c Compare December 29, 2021 09:39
@Kobzol
Copy link
Contributor Author

Kobzol commented Dec 29, 2021

FWIW, I also tried to go through all Rustdoc clean structures and reduce their sizes by converting all Vec<T> to Box<[T]>, which reduces 24 bytes to 16 bytes.

Sadly, it seems that it increased RSS instead of reducing it, probably because in most cases the Vec still has to be created first, and also because into_boxed_slice can reallocate, which is unfortunate. Something like https://docs.rs/thin-slice/latest/thin_slice/ could be used to reduce the size further to 8 bytes, but I'm afraid that the problems with boxing the slice will prevail.

@cjgillot
Copy link
Contributor

r? rust-lang/rustdoc

camelid
camelid requested changes Dec 29, 2021
View reviewed changes
@camelid
Copy link
Member

camelid commented Dec 29, 2021

r? @camelid

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rust-highfive rust-highfive assigned camelid and unassigned CraftSpider Dec 29, 2021
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 29, 2021
@bors
Copy link
Collaborator

bors commented Dec 29, 2021

⌛ Trying commit 07c9b8c8b5a16cbcf2e4866c2daf3683c0ac78f3 with merge eb265fcfa65559eb237cc30f1c89b67ffdabd032...

@bors
Copy link
Collaborator

bors commented Dec 30, 2021

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

@rust-timer
Copy link
Collaborator

Queued eb265fcfa65559eb237cc30f1c89b67ffdabd032 with parent 78fd0f6, future comparison URL.

@camelid camelid 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 30, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (eb265fcfa65559eb237cc30f1c89b67ffdabd032): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

@rustbot 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 30, 2021
@camelid
Copy link
Member

camelid commented Dec 30, 2021

Small instructions count improvements and mostly moderate improvements on max-rss (with a few regressions, but perhaps spurious).

@camelid camelid removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 30, 2021
@Kobzol Kobzol force-pushed the rustdoc-bindings-thin-vec branch from 07c9b8c to cde982f Compare December 31, 2021 10:10
@Kobzol Kobzol force-pushed the rustdoc-bindings-thin-vec branch from cde982f to 3d8d3f1 Compare January 1, 2022 10:29
@camelid camelid 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 Jan 1, 2022
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 2, 2022
camelid
camelid approved these changes Jan 2, 2022
View reviewed changes
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

Thanks!

@camelid
Copy link
Member

camelid commented Jan 2, 2022

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Jan 2, 2022

📌 Commit 3d8d3f1 has been approved by camelid

@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 Jan 2, 2022
@bors
Copy link
Collaborator

bors commented Jan 2, 2022

⌛ Testing commit 3d8d3f1 with merge d01a8cb3e60ba71ee5f0601c3cc48d9baf96f20b...

@bors
Copy link
Collaborator

bors commented Jan 3, 2022

💥 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 Jan 3, 2022
@camelid
Copy link
Member

camelid commented Jan 3, 2022

@bors retry

@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 Jan 3, 2022
@bors
Copy link
Collaborator

bors commented Jan 3, 2022

⌛ Testing commit 3d8d3f1 with merge b5da808...

@bors
Copy link
Collaborator

bors commented Jan 3, 2022

☀️ Test successful - checks-actions
Approved by: camelid
Pushing b5da808 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 3, 2022
@bors bors merged commit b5da808 into rust-lang:master Jan 3, 2022
@rustbot rustbot added this to the 1.59.0 milestone Jan 3, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b5da808): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

@rustbot label: -perf-regression

@Kobzol Kobzol deleted the rustdoc-bindings-thin-vec branch February 22, 2022 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid approved these changes

Assignees

@camelid camelid

Labels
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-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.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.
9 participants
@Kobzol @rust-highfive @rust-log-analyzer @cjgillot @camelid @rust-timer @bors @CraftSpider @rustbot