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

rustc_serialize: specialize opaque encoding and decoding of some u8 sequences #80115

Merged
merged 3 commits into from
Jan 2, 2021

Conversation

tgnottingham
Copy link
Contributor

This specializes encoding and decoding of some contiguous u8 sequences to use a more efficient implementation. The default implementations process each u8 individually, but that isn't necessary for the opaque encoder and decoder. The opaque encoding for u8s is a no-op, so we can just copy entire sequences as-is, rather than process them byte by byte.

This also changes some encode and decode implementations for contiguous sequences to forward to the slice and vector implementations, so that they can take advantage of the new specialization when applicable.

@rust-highfive
Copy link
Collaborator

r? @estebank

(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
@tgnottingham
Copy link
Contributor Author

tgnottingham commented Dec 17, 2020

@rustbot label T-compiler I-compiletime

This mainly optimizes for the ctfe-stress-4 incr-unchanged benchmark, which is dominated by serialization cost.

@rustbot rustbot added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 17, 2020
@wesleywiser
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Contributor

bors commented Dec 17, 2020

⌛ Trying commit 2a06635df1b69708887dbcc609e4701b11cbb672 with merge fb6802e125edaf831dc7db84a5aa3110ac47f134...

@bors
Copy link
Contributor

bors commented Dec 17, 2020

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

@rust-timer
Copy link
Collaborator

Queued fb6802e125edaf831dc7db84a5aa3110ac47f134 with parent d23e084, 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 17, 2020
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (fb6802e125edaf831dc7db84a5aa3110ac47f134): 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 18, 2020
@bjorn3
Copy link
Member

bjorn3 commented Dec 18, 2020

ctfe-stress-4 shows major improvements. ctfe-stress-4-check even shows an 86% improvement for incr-unchanged. Other than that there are slight (<1%) improvements and no regressions.

@tgnottingham tgnottingham force-pushed the specialize_opaque_u8_sequences branch from 2a06635 to be79f49 Compare January 2, 2021 06:49
@tgnottingham
Copy link
Contributor Author

Rebased and fixed conflict.

By the way, this is kind of a "soft" blocker on #80463 (only for performance reasons).

@oli-obk
Copy link
Contributor

oli-obk commented Jan 2, 2021

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jan 2, 2021

📌 Commit be79f49 has been approved by oli-obk

@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, 2021
@bors
Copy link
Contributor

bors commented Jan 2, 2021

⌛ Testing commit be79f49 with merge 929f66a...

@bors
Copy link
Contributor

bors commented Jan 2, 2021

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 929f66a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 2, 2021
@bors bors merged commit 929f66a into rust-lang:master Jan 2, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 2, 2021
@tgnottingham tgnottingham deleted the specialize_opaque_u8_sequences branch January 20, 2021 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

9 participants