Repository navigation
Conversation
MariusBgm
changed the title
MariusBgm
marked this pull request as ready for review
September 16, 2026 09:11
MariusBgm
added
the
status: ready for review
Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
Reusable receive descriptor — VAsioConnection now assigns into a member ServiceDescriptor instead of constructing a fresh one per received message. std::map assignment reuses nodes and std::string assignment reuses buffers, so the supplemental-data allocations disappear. Safe because only the io thread reaches it and dispatch is synchronous. Pooled serialization buffer — a thread-local free list in MessageBuffer. The writing constructor takes a buffer from it; MakeSendItem hands it back once a small message has been copied inline. Thread-local means no synchronization, and a buffer acquired on one thread and recycled on another just migrates between lists. Pooled receive blob — AcquireReceiveBlob reuses a blob only when its use_count() == 1, which is precisely the condition that no deserialized payload still aliases it. Self-correcting: if a payload somehow outlives dispatch, that entry is skipped and a fresh blob is allocated. Both pools are capped (32 buffers / 8 blobs, 64 KB each) so large messages don't cause hoarding. Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
Two independent findings from profiling the small message path with callgrind and DHAT, 400,000 messages of 10 bytes between two participants. asio copies a buffer sequence into the async operation, which DHAT attributed as one allocation of 16 bytes per message. A single buffer is stored in the operation by value instead, so AsyncWriteSome and AsyncReadSome now take that path when the sequence holds exactly one buffer. That is the common case: a small message is written as one contiguous buffer, and the receive ring only needs a second buffer when its free region wraps. AcquireReceiveBlob resized a pooled blob to the exact message size, value initializing bytes that RingBuffer::Read overwrites immediately afterwards. Pooled blobs now only grow and stay at their high water size, and the caller reads and views exactly the message's bytes. This part is a regression from the commit that introduced the pool. Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
- replace thread_local receive blob pool with ReceiveBlobPool owned by VAsioConnection - add Experimental.UseReceiveBufferPool (default true) to disable pooling for diagnosis - reuse best-fitting blob, cap slack at 4 KiB, acquire fence after use_count check - add VAsioPeer tests: inline/owned/shared send items, partial writes, aggregation, receive paths - add ReceiveBlobPool, VAsioProxyPeer and serdes aliasing / RPC wire format tests - add fan-out ITests: pubsub 1:3 (aggregation, pool off), RPC 1:2, large Ethernet frames, mixed direct/proxied subscribers - use non-uniform payloads in pubsub ITest harness - document UseReceiveBufferPool Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
MariusBgm
force-pushed
the
dev/zero_copy_opt
branch
from
September 25, 2026 06:11
a59dd7d to
2f8b749
Compare
Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
snps-fricke
approved these changes
Oct 8, 2026
snps-fricke
left a comment
snps-fricke
left a comment
Collaborator
There was a problem hiding this comment.
Really nice work, the fan-out design is clean and very well tested. If anything, I'd reiterate on the lifetime of receive buffers and see if that path can be simplified.
| bool operator==(const Experimental& lhs, const Experimental& rhs) | ||
| { | ||
| return lhs.timeSynchronization == rhs.timeSynchronization && lhs.metrics == rhs.metrics; | ||
| return lhs.timeSynchronization == rhs.timeSynchronization && lhs.metrics == rhs.metrics |
Collaborator
There was a problem hiding this comment.
Would be nice to replace these with the default comparisons in C++20
| { | ||
| public: | ||
| //! Maximum number of blobs held. | ||
| static constexpr size_t MaxEntries{8}; |
Collaborator
There was a problem hiding this comment.
I think multiple entries are only needed if references are retained beyond the OnSocketData call in VAsioPeer. Is that actually the case?
As an experiment, I asserted that none of the use counts are > 1 when Acquire() is called, and it seems at least the tests don't do this.
If that's actually true, could we save this complexity? Maybe VAsioConnection::_receiveBlobPool could become a single reused buffer, then.
| { | ||
| // NB: use_count() is a relaxed load. The fence orders the writes into the reused blob after | ||
| // the reads of whichever thread dropped the second-to-last reference to it. | ||
| std::atomic_thread_fence(std::memory_order_acquire); |
Collaborator
There was a problem hiding this comment.
I'd like to better understand if this is strictly required today. As far as I can tell, references can only be dropped on the I/O thread since the API hands out non-owning Spans.
Regardless, its cost is probably negligible so it might be worth keeping if this ever changes.
| } | ||
|
|
||
| // NB: prefer the smallest free blob that holds the message within the slack, so that a mix of | ||
| // message sizes settles into blobs of matching sizes instead of reallocating back and forth. |
Collaborator
There was a problem hiding this comment.
How would these continuous reallocations be caused? Because blobs only ever grow, shouldn't it converge to a state where all blobs are large enough to contain any message?
snps-fiodorov
removed
the
status: ready for review
KonradBreitsprecherBkd
added
the
status: approved
This branch has not been deployed
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
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.
Three profiling-driven optimizations on top of #423. No API, ABI or wire format change.
dynamic_castcalls: one was a plain upcast, the other is now resolved once at receiver registration. Takes the RTTIstrcmpwith it.ServiceDescriptorper message; pool the serialization buffer and the receive blob (thread local, no locking).