Code generator atomic queues for linked queues and linked array queue variation #187
Conversation
BaseLinkedQueue: Moved the last level of padding into a dedicated class BaseLinkedAtomicQueue: Added padding for the atomic field updaters
Construction of a new LinkedQueueNode or new LinkedQueueAtomicNode is done in the base class Some parameter variable names homogenized
Exposed static methods like lvElement on AtomicReferenceArray to mimick UnsafeRefArrayAccess methods
…he unsafe variant
I've replaced direct accesses to the buffer with the soElement/lvElement/etc style method accessors like the unsafe variants. I've moved the direct field accessors, e.g. soProducerIndex to the padded classes.
that are specific to the buffer data type
| @@ -46,6 +47,8 @@ | |||
| import com.github.javaparser.ast.visitor.VoidVisitorAdapter; | |||
|
|
|||
| public final class JavaParsingAtomicArrayQueueGenerator extends VoidVisitorAdapter<Void> { | |||
There was a problem hiding this comment.
Changes to this class were largely refactoring to try to stay similar to the linked queue generator
| @@ -157,11 +171,6 @@ public final boolean isEmpty() { | |||
| return lvConsumerNode() == lvProducerNode(); | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
Moved to end of class to help in diffing during dev
| protected long producerIndex; | ||
|
|
||
| @Override | ||
| public final long lvProducerIndex() |
There was a problem hiding this comment.
These accessor methods were promoted to the parent padding class to be consistent with the array queue implementations
| * This method assumes index is actually (index << 1) because lower bit is used for resize. This is | ||
| * compensated for by reducing the element shift. The computation is constant folded, so there's no cost. | ||
| */ | ||
| private static long modifiedCalcElementOffset(long index, long mask) |
There was a problem hiding this comment.
This was moved into a util method, LinkedArrayQueueUtil.modifiedCalcElementOffset
| @@ -407,51 +452,11 @@ private E newBufferPeek(E[] nextBuffer, long index) | |||
| private long newBufferAndOffset(E[] nextBuffer, long index) | |||
| { | |||
| consumerBuffer = nextBuffer; | |||
| consumerMask = (nextBuffer.length - 2) << 1; | |||
| consumerMask = (length(nextBuffer) - 2) << 1; | |||
There was a problem hiding this comment.
This is defined in LinkedArrayQueueUtil.length as a sneaky way of simplying the generation process. i.e. for atomics all it has to do is import LinkedArrayAtomicQueueUtil.length and this will "just work"(tm)
| return nextBuffer; | ||
| } | ||
|
|
||
| private long nextArrayOffset(E[] curr) |
There was a problem hiding this comment.
This was moved into LinkedArrayQueueUtil
| @@ -52,58 +49,48 @@ public SpscChunkedArrayQueue(int chunkSize, int capacity) | |||
| } | |||
|
|
|||
| @Override | |||
| public int capacity() | |||
There was a problem hiding this comment.
Moved to the end of this class
| if (casProducerIndex(pIndex, pIndex + 2)) { | ||
| break; | ||
| } | ||
| } | ||
| // INDEX visible before ELEMENT, consistent with consumer expectation | ||
| final int offset = modifiedCalcElementOffset(pIndex, mask); | ||
| buffer.lazySet(offset, e); |
There was a problem hiding this comment.
Swapped this to using the soElement method in the util classes for easy transformation by the generator
| } | ||
|
|
||
| private void soProducerIndex(long v) { | ||
| producerIndex.set(v); |
There was a problem hiding this comment.
This seemed wrong.. the generator outputs lazySet. I think that's correct.
| } | ||
|
|
||
| private void soConsumerIndex(long v) { | ||
| consumerIndex.set(v); |
There was a problem hiding this comment.
This seemed wrong.. the generator outputs lazySet. I think that's correct.
| } | ||
|
|
||
| private void soProducerLimit(long v) { | ||
| producerLimit.set(v); |
There was a problem hiding this comment.
This seemed wrong.. the generator outputs lazySet. I think that's correct.
| @@ -104,6 +180,40 @@ public BaseMpscLinkedAtomicArrayQueue(final int initialCapacity) { | |||
| } | |||
|
|
|||
| @Override | |||
| public final int size() { | |||
There was a problem hiding this comment.
This, and a few other methods in the class were moved around but unchanged to make it easier to diff unsafe vs atomics during dev
| soElement(curr, nextArrayOffset(curr), next); | ||
| } | ||
|
|
||
| private void soElement(AtomicReferenceArray curr, int i, Object e) { |
| adjustLookAheadStep(chunkCapacity); | ||
| soProducerIndex(0L);// serves as a StoreStore barrier to support correct publication |
There was a problem hiding this comment.
Atomics version had this but unsafe used a final field so this disappears
| int chunkCapacity = Math.max(Pow2.roundToPowerOfTwo(chunkSize), 16); | ||
| long mask = chunkCapacity - 1; | ||
| AtomicReferenceArray<E> buffer = allocate(chunkCapacity + 1); | ||
| producerBuffer = buffer; | ||
| producerMask = mask; | ||
| consumerBuffer = buffer; | ||
| consumerMask = mask; | ||
| producerBufferLimit = mask - 1; // we know it's all empty to start with | ||
| soProducerIndex(0L); |
There was a problem hiding this comment.
ATTENTION!: atomics had barrier this but unsafe version didn't. However this class nor it's parents write to a final field. This would have been an existing bug in the unsafe version, could you look this over and let me know your thoughts.
kay
force-pushed
the
homogenize_atomic_linked_queues
branch
from
d6f62d4
to
15e1f6a
Compare
| producerQueueLimit = maxQueueCapacity; | ||
| soProducerIndex(0L);// serves as a StoreStore barrier to support correct publication |
There was a problem hiding this comment.
Think this is fine as maxQueueCapacity is final and this barrier will be inserted then
There was a problem hiding this comment.
This volatile/ordered write at the end of construction as a substitute for final field safe publication guarantees does not work sadly (see Shipilev JMM thingy). I think this was a misguided effort in any case, queues should be safely published before used, which is generally the case anyhow.
There was a problem hiding this comment.
You're referring to http://altair.cs.oswego.edu/pipermail/concurrency-interest/2013-November/011951.html ? i.e. that technically by JMM wording ctors that contain volatile writes is not the same as a final write in terms of safe publication?
As part of doing this I also implemented MessagePassingQueue for all variations. The reason for this was simply that I had to anyway (path of least resistance). However I can break that out into a separate PR if that makes more sense.
Queue classes now generated: