RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v9]

[Viktor Klang]

On Fri, 21 Jul 2023 12:31:07 GMT, Doug Lea <dl at 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