On Fri, 21 Jul 2023 12:31:07 GMT, Doug Lea <d...@openjdk.org> wrote:
>> This update addresses performance issues across both LinkedTransferQueue and
>> SynchronousQueue by creating a common basis for implementation across them
>> (mainly in LinkedTransferQueue). Pasting from internal doc summary of
>> changes:
>> * * Class DualNode replaces Qnode, with fields and methods
>> * that apply to any match-based dual data structure, and now
>> * usable in other j.u.c classes. in particular, SynchronousQueue.
>> * * Blocking control (in class DualNode) accommodates
>> * VirtualThreads and (perhaps virtualized) uniprocessors.
>> * * All fields of this class (LinkedTransferQueue) are
>> * default-initializable (to null), allowing further extension
>> * (in particular, SynchronousQueue.Transferer)
>> * * Head and tail fields are lazily initialized rather than set
>> * to a dummy node, while also reducing retries under heavy
>> * contention and misorderings, and relaxing some accesses,
>> * requiring accommodation in many places (as well as
>> * adjustments in WhiteBox tests).
>
> Doug Lea has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Address review comments
src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line
639:
> 637: else {
> 638: if (p != h && cmpExHead(h, p) == h)
> 639: h.next = h; // h.selfLink();
@DougLea Too expensive with the call? 🤔
src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line
667:
> 665: else if (cmpExHead(c, p) != c)
> 666: return false;
> 667: if (c != null)
@DougLea If this `if` is not related to the above if-tree, I'd put a newline
between them. (Also makes sense to add a newline before the last `return` as
well (to indicate that it is standalone from the if)
src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line
726:
> 724: else if (p == h) // traverse past header
> 725: p = q;
> 726: else if ((u = cmpExHead(h, q)) != h)
@DougLea Always nice to see `compareAndExchange` being used :)
src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line
896:
> 894: for (DualNode p = (pred == null) ? head : pred.next, c = p;
> 895: p != null; ) {
> 896: boolean isData = p.isData;
@DougLea Are you finding that manual hoisting of reads to final members has a
perf edge? 🤔
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270704711
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270707826
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270709052
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270711614
