Fix peek/relaxedPeek implementation of MpmcArrayQueue.#295
Fix peek/relaxedPeek implementation of MpmcArrayQueue.#295hl845740757 wants to merge 88 commits into
Conversation
This is fixing MpqBurstCost and QueueBurstCost multi-consumer scenario and will make both to look similar.
Fixes JCTools#272 BurstCost aren't handling multi-consumers correctly
+ javadoc + tweak visibility of methods
…CTools#281) QueueSanityTest::testSize with chunkSize = 1 and 1 recycled chunks was failing with a blocked poll because it wasn't handling ccChunkIndex == ciChunkIndex as an isFirstElementOfNextChunk case
Though it's already package private, but anyhow
Avoids the field re-ordering introduced by JDK 15
Also handle negative edge cases for index queues (fix issue for Spsc) Fixes JCTools#292
Pull Request Test Coverage Report for Build 661
💛 - Coveralls |
|
Thanks for the interesting test case. Can you add it to the existing test suite so all the queues are covered? |
| // only return null if queue is empty | ||
| // Volatile Mode: Loading element must be completed before loading pIndex. | ||
| e = lvRefElement(buffer, calcCircularRefElementOffset(cIndex, mask)); | ||
| pIndex = lvProducerIndex(); |
There was a problem hiding this comment.
The original solution relying on the matching sequence is more in line with how poll works and the spirit of this queue altogether. The idea here is to avoid as much as possible cache misses/traffic induced by loading the producer index from the consumer threads.
There was a problem hiding this comment.
The sequence could be read twice to check if the read element is stable, similarly to what a stamped lock does
There was a problem hiding this comment.
Stable value is unnecessary
| long cIndex = lvConsumerIndex(); | ||
| // Volatile Mode: Loading element must be completed before loading pIndex. | ||
| E e = lvRefElement(buffer, calcCircularRefElementOffset(cIndex, mask)); | ||
| long pIndex = lvProducerIndex(); |
There was a problem hiding this comment.
relaxed peek should be implemented without loading the producer index at all.
|
I think this is now resolved |
Summary
Poll/relaxedPoll/Peek/RelaxedPeek should not return an older value.
ConsumerC: prepare to load element (currentConsumerIndex)
ConsumerA: poll the element (currentConsumerIndex)
Producer: offer the element (currentConsumerIndex + capaity)
Producer: complete store element
ConsumerC: complete load element (currentConsumerIndex + capaity)
ConsumerC expects to load currentConsumerIndex, but loads (currentConsumerIndex + capaity).
The implementation of SpmcArrayQueue has the same bug.