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

Derive Rustc{En,De}codable for TokenStream. #65641

Merged

Conversation

nnethercote
Copy link
Contributor

TokenStream used to be a complex type, but it is now just a newtype
around a Lrc<Vec<TreeAndJoint>>. Currently it uses custom encoding
that discards the IsJoint and custom decoding that adds NonJoint
back in for every token tree. This requires building intermediate
Vec<TokenTree>s.

This commit makes TokenStream derive Rustc{En,De}codable. This
simplifies the code, and avoids the creation of the intermediate
vectors, saving up to 3% on various benchmarks. It also changes the AST
JSON output in one test.

r? @petrochenkov

`TokenStream` used to be a complex type, but it is now just a newtype
around a `Lrc<Vec<TreeAndJoint>>`. Currently it uses custom encoding
that discards the `IsJoint` and custom decoding that adds `NonJoint`
back in for every token tree. This requires building intermediate
`Vec<TokenTree>`s.

This commit makes `TokenStream` derive `Rustc{En,De}codable`. This
simplifies the code, and avoids the creation of the intermediate
vectors, saving up to 3% on various benchmarks. It also changes the AST
JSON output in one test.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 20, 2019
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Oct 20, 2019

⌛ Trying commit c290293 with merge 1b0e1f524644251dc9f642f247055f5af49ddddd...

@bors
Copy link
Contributor

bors commented Oct 21, 2019

☀️ Try build successful - checks-azure
Build commit: 1b0e1f524644251dc9f642f247055f5af49ddddd (1b0e1f524644251dc9f642f247055f5af49ddddd)

@rust-timer
Copy link
Collaborator

Queued 1b0e1f524644251dc9f642f247055f5af49ddddd with parent 89e645a, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 1b0e1f524644251dc9f642f247055f5af49ddddd, comparison URL.

@nnethercote
Copy link
Contributor Author

The perf results show lots of instruction count wins, up to 1.5%. The wall-time results are much noisier, of course, but also show a clear improvement.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 21, 2019

📌 Commit c290293 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 21, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 22, 2019
…dable-Decodable, r=petrochenkov

Derive `Rustc{En,De}codable` for `TokenStream`.

`TokenStream` used to be a complex type, but it is now just a newtype
around a `Lrc<Vec<TreeAndJoint>>`. Currently it uses custom encoding
that discards the `IsJoint` and custom decoding that adds `NonJoint`
back in for every token tree. This requires building intermediate
`Vec<TokenTree>`s.

This commit makes `TokenStream` derive `Rustc{En,De}codable`. This
simplifies the code, and avoids the creation of the intermediate
vectors, saving up to 3% on various benchmarks. It also changes the AST
JSON output in one test.

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 23, 2019
…dable-Decodable, r=petrochenkov

Derive `Rustc{En,De}codable` for `TokenStream`.

`TokenStream` used to be a complex type, but it is now just a newtype
around a `Lrc<Vec<TreeAndJoint>>`. Currently it uses custom encoding
that discards the `IsJoint` and custom decoding that adds `NonJoint`
back in for every token tree. This requires building intermediate
`Vec<TokenTree>`s.

This commit makes `TokenStream` derive `Rustc{En,De}codable`. This
simplifies the code, and avoids the creation of the intermediate
vectors, saving up to 3% on various benchmarks. It also changes the AST
JSON output in one test.

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 23, 2019
…dable-Decodable, r=petrochenkov

Derive `Rustc{En,De}codable` for `TokenStream`.

`TokenStream` used to be a complex type, but it is now just a newtype
around a `Lrc<Vec<TreeAndJoint>>`. Currently it uses custom encoding
that discards the `IsJoint` and custom decoding that adds `NonJoint`
back in for every token tree. This requires building intermediate
`Vec<TokenTree>`s.

This commit makes `TokenStream` derive `Rustc{En,De}codable`. This
simplifies the code, and avoids the creation of the intermediate
vectors, saving up to 3% on various benchmarks. It also changes the AST
JSON output in one test.

r? @petrochenkov
bors added a commit that referenced this pull request Oct 24, 2019
Rollup of 12 pull requests

Successful merges:

 - #64178 (More Clippy fixes for alloc, core and std)
 - #65144 (Add Cow::is_borrowed and Cow::is_owned)
 - #65193 (Lockless LintStore)
 - #65479 (Add the `matches!( $expr, $pat ) -> bool` macro)
 - #65518 (Avoid ICE when checking `Destination` of `break` inside a closure)
 - #65583 (rustc_metadata: use a table for super_predicates, fn_sig, impl_trait_ref.)
 - #65641 (Derive `Rustc{En,De}codable` for `TokenStream`.)
 - #65648 (Eliminate `intersect_opt`.)
 - #65657 (Remove `InternedString`)
 - #65691 (Update E0659 error code long explanation to 2018 edition)
 - #65696 (Fix an issue with const inference variables sticking around under Chalk + NLL)
 - #65704 (relax ExactSizeIterator bound on write_bytes)

Failed merges:

r? @ghost
@bors bors merged commit c290293 into rust-lang:master Oct 24, 2019
@nnethercote nnethercote deleted the derive-TokenStream-Encodable-Decodable branch October 25, 2019 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants