Reduce allocation counts in a couple of places #1571
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #1571 +/- ##
==========================================
- Coverage 96.41% 96.41% -0.01%
==========================================
Files 75 75
Lines 15524 15517 -7
==========================================
- Hits 14968 14961 -7
Misses 556 556
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
a1665b7 to
85d0c5d
Compare
This saves some allocations and copies of relatively large data.
split_at always creates a new Vec, but drain just moves bytes within the existing Vec.
85d0c5d to
c4d279b
Compare
djc
approved these changes
Nov 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Messing around with heaptrack in #1563, I identified a couple of places with relatively high allocation counts when running
simpleclient(modified to do several connections in a row).In CertificatePayloadTls13::convert, we were unnecessarily doing a clone. Fixed this by converting that function to take
self, and the two callsites to userequire_handshake_msg_move!instead ofrequire_handshake_msg!.In
ChunkVecBuffer::consume, when a chunk is partially consumed, we were calling Vec::split_off, which always "Returns a newly allocated vector." Instead, use Vec::drain, which reuses the current allocation.In local testing, the change to
ChunkVecBuffer::consumeappeared to reduce allocations by almost 10%. I'll be curious to see if the CI run shows actual runtime performance.