create scuffle-bytes-util #241
Merged
Conversation
This file contains 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
|
Build broken due to rust-lang/rust#135235 |
TroyKomodo
commented
Jan 9, 2025
TroyKomodo
commented
Jan 9, 2025
Comment on lines
10
to
21
| /// Extracts the remaining bytes from the cursor returning. | ||
| /// | ||
| /// This does not do a copy of the bytes, and is O(1) time. | ||
| /// | ||
| /// This is the same as `BytesCursor::extract_bytes(self.remaining())`. | ||
| fn extract_remaining(&mut self) -> Bytes; | ||
|
|
||
| /// Extracts a bytes from the cursor. | ||
| /// | ||
| /// This does not do a copy of the bytes, and is O(1) time. | ||
| /// Returns an error if the size is greater than the remaining bytes. | ||
| fn extract_bytes(&mut self, size: usize) -> io::Result<Bytes>; |
There was a problem hiding this comment.
Open to naming suggestions for these methods too. I dont really like them at the moment.
lennartkloock
approved these changes
Jan 9, 2025
lennartkloock
requested changes
Jan 9, 2025
which one? |
lennartkloock
approved these changes
Jan 9, 2025
TroyKomodo
force-pushed
the
troy/remove-bytesio
branch
from
99b1c08 to
f91b832
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #241 +/- ##
==========================================
- Coverage 50.43% 50.24% -0.20%
==========================================
Files 198 196 -2
Lines 16269 16202 -67
==========================================
- Hits 8206 8141 -65
+ Misses 8063 8061 -2
|
lennartkloock
requested changes
Jan 9, 2025
lennartkloock
approved these changes
Jan 10, 2025
|
?brawl merge |
|
📌 Commit b1a9a32 has been approved and added to the merge queue. Requested by: @lennartkloock Approved by: @lennartkloock |
scuffle-brawl bot
added a commit
that referenced
this pull request
Jan 10, 2025
create scuffle-bytes-util Rename `bytesio` to `scuffle-bytes-util`. We also remove the wrapper types we had and optimize the `Cursor<Bytes>` usage. A lot of what we did with the wrapper types was essentially what `BytesMut` does and what `Vec<u8>` does, which is why most of the usage has been replaced with that. These changes allow for the code to have fewer copies in the rtmp implementation (I will look to further clean up the tests and code there in a future PR). The old rtmp flow was such: - copy bytes from wire into a temporary buffer - copy that buffer into the chunk decoder (or handshake buffer) buffer - parse chunks/handshake - create a temporary buffer to write back result - copy result from tmp buffer onto wire. The new flow looks like: - copy bytes from wire into a buffer - parse chunks/handshake - write result into a buffer - copy result onto the wire The newer example reuses existing buffers; so each connection will store additional memory (roughly 10kb/connection). As apposed to creating a buffer each time we want to read or write we just reuse an existing one. We also remove the additional copy to the handshake or chunk decode buffers by having them all share the same buffer (the read buffer). CLOUD-23 Requested-by: lennartkloock <[email protected]> Reviewed-by: lennartkloock <[email protected]>
scuffle-brawl
bot
added
B-brawl-merging
and removed
B-brawl-queued
B-brawl-merging
labels
|
🚨 PR has changed while a merge was in progress, cancelling the merge job. |
|
?brawl retry |
|
📌 Commit b1a9a32 has been approved and added to the merge queue. Requested by: @TroyKomodo Approved by: @lennartkloock |
|
🎉 Build successful! Approved by: @lennartkloock |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Rename
bytesiotoscuffle-bytes-util. We also remove the wrapper types we had and optimize theCursor<Bytes>usage. A lot of what we did with the wrapper types was essentially whatBytesMutdoes and whatVec<u8>does, which is why most of the usage has been replaced with that.These changes allow for the code to have fewer copies in the rtmp implementation (I will look to further clean up the tests and code there in a future PR).
The old rtmp flow was such:
The new flow looks like:
The newer example reuses existing buffers; so each connection will store additional memory (roughly 10kb/connection). As apposed to creating a buffer each time we want to read or write we just reuse an existing one.
We also remove the additional copy to the handshake or chunk decode buffers by having them all share the same buffer (the read buffer).
CLOUD-23