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.

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

Encode less metadata for proc-macro crates #76897

Merged
merged 1 commit into from
Sep 26, 2020
Merged

Encode less metadata for proc-macro crates #76897

merged 1 commit into from
Sep 26, 2020

Conversation

Aaron1011
Copy link
Member

Currently, we serialize the same crate metadata for proc-macro crates as
we do for normal crates. This is quite wasteful - almost none of this
metadata is ever used, and much of it can't even be deserialized (if it
contains a foreign CrateNum).

This PR changes metadata encoding to skip encoding the majority of crate
metadata for proc-macro crates. Most of the Lazy<[T]> fields are left
completetly empty, while the non-lazy fields are left as-is.

Additionally, proc-macros now have a def span that does not include
their body. This was done for normal functions in #75465, but was missed
for proc-macros.

As a result of this PR, we should only ever encode local CrateNums
when encoding proc-macro crates. I've added a specialized serialization
impl for CrateNum to assert this.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Sep 19, 2020
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Sep 19, 2020

⌛ Trying commit c0a1bccc2956267914c3f843acf0552cf100025b with merge 8364b62485652df7e6f55f5d5b925082ad5b9094...

@bors
Copy link
Contributor

bors commented Sep 19, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 8364b62485652df7e6f55f5d5b925082ad5b9094 (8364b62485652df7e6f55f5d5b925082ad5b9094)

@rust-timer
Copy link
Collaborator

Queued 8364b62485652df7e6f55f5d5b925082ad5b9094 with parent bbc6774, future comparison URL.

@jyn514 jyn514 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-metadata Area: Crate metadata labels Sep 19, 2020
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (8364b62485652df7e6f55f5d5b925082ad5b9094): 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

@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Sep 19, 2020

⌛ Trying commit 2341efee2714b050e7144d1a6e5c678f4c6bc11e with merge 9d18afcee1a0378a6656c876de82486d528525e7...

@jyn514 jyn514 added the I-compiletime Issue: Problems and improvements with respect to compile times. label Sep 19, 2020
@bors
Copy link
Contributor

bors commented Sep 19, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 9d18afcee1a0378a6656c876de82486d528525e7 (9d18afcee1a0378a6656c876de82486d528525e7)

@rust-timer
Copy link
Collaborator

Queued 9d18afcee1a0378a6656c876de82486d528525e7 with parent c6ab8e5, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (9d18afcee1a0378a6656c876de82486d528525e7): 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

@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Sep 19, 2020

⌛ Trying commit bc917942284b2721f542ca4348a8706999d2d01a with merge a16f40b1158cae86e56721c06794efa04f3918ce...

@bors
Copy link
Contributor

bors commented Sep 19, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: a16f40b1158cae86e56721c06794efa04f3918ce (a16f40b1158cae86e56721c06794efa04f3918ce)

@Aaron1011
Copy link
Member Author

cc @rust-lang/infra A perf run didn't get queued for some reason

@Aaron1011
Copy link
Member Author

@rust-timer build a16f40b1158cae86e56721c06794efa04f3918ce

@rust-timer
Copy link
Collaborator

Queued a16f40b1158cae86e56721c06794efa04f3918ce with parent 59fb88d, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (a16f40b1158cae86e56721c06794efa04f3918ce): 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

@petrochenkov
Copy link
Contributor

r=me with nits addressed.

@petrochenkov petrochenkov 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 Sep 26, 2020
@Aaron1011 Aaron1011 force-pushed the feature/min-proc-macro-metadata branch from bc91794 to be40cd3 Compare September 26, 2020 16:25
@Aaron1011
Encode less metadata for proc-macro crates
b965356 
Currently, we serialize the same crate metadata for proc-macro crates as
we do for normal crates. This is quite wasteful - almost none of this
metadata is ever used, and much of it can't even be deserialized (if it
contains a foreign `CrateNum`).

This PR changes metadata encoding to skip encoding the majority of crate
metadata for proc-macro crates. Most of the `Lazy<[T]>` fields are left
completetly empty, while the non-lazy fields are left as-is.

Additionally, proc-macros now have a def span that does not include
their body. This was done for normal functions in rust-lang#75465, but was missed
for proc-macros.

As a result of this PR, we should only ever encode local `CrateNum`s
when encoding proc-macro crates. I've added a specialized serialization
impl for `CrateNum` to assert this.
@Aaron1011 Aaron1011 force-pushed the feature/min-proc-macro-metadata branch from be40cd3 to b965356 Compare September 26, 2020 16:26
@Aaron1011
Copy link
Member Author

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Sep 26, 2020

📌 Commit b965356 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 26, 2020
@bors
Copy link
Contributor

bors commented Sep 26, 2020

⌛ Testing commit b965356 with merge 623fb90...

@bors
Copy link
Contributor

bors commented Sep 26, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: petrochenkov
Pushing 623fb90 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 26, 2020
@bors bors merged commit 623fb90 into rust-lang:master Sep 26, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 26, 2020
@Aaron1011 Aaron1011 mentioned this pull request Oct 6, 2020
Open
@petrochenkov petrochenkov mentioned this pull request Nov 23, 2020
Merged
@compiler-errors compiler-errors mentioned this pull request Mar 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels
A-metadata Area: Crate metadata I-compiletime Issue: Problems and improvements with respect to compile times. 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.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.
7 participants
@Aaron1011 @rust-highfive @rust-timer @bors @petrochenkov @jyn514 @rustbot