Skip to content
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.

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

Remove DefPath from Visibility and calculate it on demand #80099

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 17, 2020

Depends on #80090 and should not be merged before. Helps with #79103 and #76382.

cc #80014 (comment) - @nnethercote I figured it out! It was simpler than I expected :)

This brings the size of clean::Visibility down from 40 bytes to 8.

Note that this does not remove clean::Visibility, even though it's now basically the same as ty::Visibility, because the Invsible variant means something different from Inherited and I thought it would be be confusing to merge the two. See the new comments on impl Clean for ty::Visibility for details.

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Dec 17, 2020
@rust-highfive
Copy link
Contributor

r? @ollie27

(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 17, 2020
@jyn514 jyn514 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 17, 2020
@rust-log-analyzer

This comment has been minimized.

@jyn514 jyn514 force-pushed the visibility-on-demand branch from b37312d to 7c75770 Compare December 17, 2020 01:25
@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 18, 2020
@jyn514 jyn514 force-pushed the visibility-on-demand branch from 7c75770 to 4b4a260 Compare December 18, 2020 16:29
@jyn514 jyn514 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 Dec 18, 2020
@rust-log-analyzer

This comment has been minimized.

@jyn514 jyn514 force-pushed the visibility-on-demand branch from 4b4a260 to a2fb4b9 Compare December 18, 2020 17:30
@jyn514
Copy link
Member Author

jyn514 commented Dec 22, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Collaborator

bors commented Dec 22, 2020

⌛ Trying commit a2fb4b9 with merge fbe5dedd7d90604c50736a818a6ea08e195a377e...

@bors
Copy link
Collaborator

bors commented Dec 22, 2020

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

@rust-timer
Copy link
Collaborator

Queued fbe5dedd7d90604c50736a818a6ea08e195a377e with parent 353f3a3, future comparison URL.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 22, 2020
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (fbe5dedd7d90604c50736a818a6ea08e195a377e): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 23, 2020
@jyn514
Copy link
Member Author

jyn514 commented Dec 23, 2020

-.5% on instructions, -.6% on max-rss.

@bors r=GuillaumeGomez rollup=never

@bors
Copy link
Collaborator

bors commented Dec 23, 2020

📌 Commit a2fb4b9 has been approved by GuillaumeGomez

@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 Dec 23, 2020
@bors
Copy link
Collaborator

bors commented Dec 23, 2020

⌛ Testing commit a2fb4b9 with merge 28d73a3...

@bors
Copy link
Collaborator

bors commented Dec 23, 2020

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 28d73a3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 23, 2020
@bors bors merged commit 28d73a3 into rust-lang:master Dec 23, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 23, 2020
@jyn514 jyn514 deleted the visibility-on-demand branch December 23, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. 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-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.

8 participants