Weaken memory ordering OneToOnRingBuffer#359
Weaken memory ordering OneToOnRingBuffer#359pveentjer wants to merge 1 commit intoaeron-io:masterfrom
Conversation
pveentjer
force-pushed
the
cleanup/OneToOneRingBuffer-relaxed-memory-ordering
branch
from
7dc72f8 to
a39fce5
Compare
| return false; | ||
| } | ||
|
|
||
| buffer.putIntRelease(lengthOffset(recordIndex), -recordLength); |
There was a problem hiding this comment.
For the actual claim (so writing the negative record length), only opaque is needed. Surrounding loads/stores do not need to be ordered. The content of the record can't be read (apart from the length) until the commit is done. The commit is performed using a release store and will make sure that all writes to the record are visible when the positive length is read by the reader.
| return recordIndex; | ||
| } | ||
|
|
||
| buffer.putIntRelease(lengthOffset(recordIndex), -recordLength); |
There was a problem hiding this comment.
Also an opaque write for the record length. We do not care for any ordering guarantees. Only when the record is committed, a release store is needed which will order the stores to the record before it.
| } | ||
|
|
||
| buffer.putIntRelease(lengthOffset(recordIndex), -recordLength); | ||
| VarHandle.releaseFence(); |
There was a problem hiding this comment.
The VarHandle.releaseFence can be dropped. THe only ordering that is needed is the release store of the (positive) record length when committing. This will order all the stores to the record before it.
| if (0 != padding) | ||
| { | ||
| buffer.putLong(0, 0L); | ||
| buffer.putIntRelease(lengthOffset(recordIndex), -padding); |
There was a problem hiding this comment.
We don't need to announce the negative length. The reading side will not read the record while the length is not positive. And there won't be any abort in this case.
| { | ||
| buffer.putLong(0, 0L); | ||
| buffer.putIntRelease(lengthOffset(recordIndex), -padding); | ||
| VarHandle.releaseFence(); |
There was a problem hiding this comment.
The releaseFence is not needed. The release store of the commit (so writing the positive length) will order the write of the message type, before it.
|
Once this PR is merged, I'll apply the same changes to the aeron_spsc_rb.c |
pveentjer
force-pushed
the
cleanup/OneToOneRingBuffer-relaxed-memory-ordering
branch
from
a39fce5 to
dd2b636
Compare
pveentjer
changed the title
pveentjer
changed the title
1 participant
The current code has excessive memory ordering and it can be reduced.