aeron
aeron copied to clipboard
[Cpp] Added tryClaim method to concurrent ringbuffers.
In the codebase my team is working on we would like to use concurrent::ringbuffers
. For us, tryClaim
interface is preferred. I tried following you're code convetion as much as possible but reused the tryClaim
implementaion for write
methods(that way tryClaim also gets tested with write
).
Unfortunately order of atomic writes for oneToOneRingBuffer
was changed. Tail
gets written first and only after does messageHeader
get written. This could affect performance.
In our implementation of spsc
channel we provide the callback to the tryClaim
method which gets called with the claimed buffer. That way tail
gets written after the messageHeader
; wanting to stay within the convention of your codebase this PR does it by introducing RingBufferClaim
class.
Edit: As per @mjpt777's suggestion refactored the code to reflect Java RingBuffer
s.
It would be best if the implementations followed the same API semantics as we have for the Java RingBuffer
s.
https://github.com/real-logic/agrona/tree/master/agrona/src/main/java/org/agrona/concurrent/ringbuffer
It would be best if the implementations followed the same API semantics as we have for the Java
RingBuffer
s.https://github.com/real-logic/agrona/tree/master/agrona/src/main/java/org/agrona/concurrent/ringbuffer
Will do
@mjpt777 could there be a review of this ?
There are two things that stand out:
- Usage of the
RecordDescriptor::makeHeader
to combinelength
andtype
fields together. We have decided that it would be better if the C++ version was aligned with the Java and C implementations, i.e. it should read/writelength
andtype
as separate fields.OneToOneRingBuffer::claimCapacity
is not aligned with the Java implementation, i.e. in Java and in C we add an additionalHEADER_LENGTH
when computing therequiredCapacity
. This allows for pre-zeroing of the next header upon claim and thus we don't need to zero the memory in theread/controlledRead
methods.Another thing to note is that the C++ and C implementations don't have the
controlledRead
API.
Will fix this probably some time next week. Thanks for the input.
Is this progressing?