protozero: let transports read directly into ProtoRingBuffer - #7264
Open
LalitMaganti wants to merge 1 commit into
Open
protozero: let transports read directly into ProtoRingBuffer#7264LalitMaganti wants to merge 1 commit into
LalitMaganti wants to merge 1 commit into
Conversation
🎨 Perfetto UI Builds
|
Member
|
I'm going to take these reviews if you don't mind, as I wrote these classes |
primiano
self-requested a review
August 28, 2026 09:17
primiano
reviewed
Aug 28, 2026
primiano
left a comment
primiano
left a comment
Member
There was a problem hiding this comment.
Very nice. LGTM % one question on what hapepend to unixd.cc, i'm confused on how that can still work.
LalitMaganti
force-pushed
the
dev/lalitm/zerocopy-1-ringbuffer
branch
from
August 28, 2026 10:19
3b19ce9 to
08af104
Compare
Member
Author
Replied, PTAL |
primiano
reviewed
Aug 28, 2026
primiano
left a comment
primiano
left a comment
Member
There was a problem hiding this comment.
much better, LG with some minor questions
LalitMaganti
force-pushed
the
dev/lalitm/zerocopy-1-ringbuffer
branch
from
August 28, 2026 15:10
08af104 to
537a808
Compare
Member
Author
|
Reworked it as you suggested. PTAL. |
LalitMaganti
force-pushed
the
dev/lalitm/zerocopy-1-ringbuffer
branch
from
August 28, 2026 15:30
537a808 to
0ab6f73
Compare
Every transport kept a staging buffer, read into it, then handed those
bytes to Append(), which copied them again. BeginWrite()/EndWrite() lets
read(2) deposit them in the ring buffer directly, so they are copied
once.
The reservation is handed out as a move-only WriteHandle that must be
consumed exactly once, by EndWrite() to keep what was written or
AbortWrite() to discard it; dropping one fails a CHECK. Holding it in a
value the compiler tracks is what makes the pattern safe to use: the
buffer refuses to recompact, grow or hand out a second reservation while
one is outstanding, so bytes cannot move under a pointer someone still
holds. Rpc::RequestHandle carries the same contract out to the
transports.
Four bugs fell out of this, three of them a reservation abandoned on a
path that returns early:
- The size of the reservation used to live in a member alongside the
pointer, and the path that gives up on a stream too long to ever form
a message returned without setting it. The caller's next EndWrite()
then failed its CHECK, so a peer could abort the process by sending
128MB of varint continuation bytes. The count now lives in the handle,
where it cannot fall out of step with the pointer it describes.
- traceconv dropped the reservation it sniffs the first chunk in
whenever the trace turned out to be compressed, and again whenever the
decompressor errored.
- RemoteTraceProcessor dropped one whenever a recv failed.
unixd additionally re-serialized each tokenized message back into its
TraceProcessorRpcStream framing and pushed it through a second ring
buffer inside Rpc. Rpc::OnRpcMessage() dispatches it as-is.
out/linux_clang_release/perfetto_benchmarks \
--benchmark_filter='ProtoRingBufferIngest|ProtoRingBufferDispatch'
ingest, 4KB reads 142 ns -> 98.3 ns
ingest, 1MB reads 38.4 us -> 19.2 us
dispatch 171 ns -> 99.1 ns
traceconv also drops a 64MB read buffer: peak RSS 62.7MB -> 13.6MB.
LalitMaganti
force-pushed
the
dev/lalitm/zerocopy-1-ringbuffer
branch
from
August 28, 2026 15:38
0ab6f73 to
fe1f2d8
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Every transport kept a staging buffer, read into it, then handed those
bytes to Append(), which copied them again. BeginWrite()/EndWrite() lets
read(2) deposit them in the ring buffer directly, so they are copied
once.
The reservation is handed out as a move-only WriteHandle that must be
consumed exactly once, by EndWrite() to keep what was written or
AbortWrite() to discard it; dropping one fails a CHECK. Holding it in a
value the compiler tracks is what makes the pattern safe to use: the
buffer refuses to recompact, grow or hand out a second reservation while
one is outstanding, so bytes cannot move under a pointer someone still
holds. Rpc::RequestHandle carries the same contract out to the
transports.
out/linux_clang_release/perfetto_benchmarks
--benchmark_filter='ProtoRingBufferIngest|ProtoRingBufferDispatch'
ingest, 4KB reads 142 ns -> 98.3 ns
ingest, 1MB reads 38.4 us -> 19.2 us
dispatch 171 ns -> 99.1 ns
traceconv also drops a 64MB read buffer: peak RSS 62.7MB -> 13.6MB.