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

[Viktor Klang]

On Fri, 30 Jun 2023 19:38:19 GMT, Doug Lea <dl at openjdk.org> wrote:

>> This now uses Thread.isVirtual to distinguish spin vs immediate block cases, enabling re-introduction of spin control from the previous version, removing anomalies like this one.
>
> Doug Lea has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix inverted test assert; improve internal documentation; simplify code

src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 144:

> 142:      * nodes, CASing from null to a data value.  (To reduce the need
> 143:      * for re-reads, we use the compareAndExchange forms of CAS for
> 144:      * pointer updates, that provide the current value to comtinue

Suggestion:

     * pointer updates, that provide the current value to continue

src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 158:

> 156:      * suffix of zero or more unmatched nodes. Note that we allow both
> 157:      * the prefix and suffix to be zero length, which in turn means
> 158:      * that we do not require a dummy header.

@DougLea Great to avoid having the dummy header 👍

src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 484:

> 482:      * be at least 4.
> 483:      */
> 484:     private static final int MAX_SLACK = 1 << 7;

@DougLea Any guidance on how to tune this parameter? 🤔

src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 570:

> 568:         boolean haveData = (e != null);
> 569:         TransferNode p;                     // current traversal node
> 570:         TransferNode s = null;              // the enqueued npde, if needed

Suggestion:

        TransferNode s = null;              // the enqueued node, if needed

src/java.base/share/classes/java/util/concurrent/SynchronousQueue.java line 133:

> 131:      *     compareAndExchange form of CAS for pointer updates to reduce
> 132:      *     memory traffic. The Fifo version accommodates lazy
> 133:      *     updates and slack as desxcribed in the LinkedTransferQueue

Suggestion:

     *     updates and slack as described in the LinkedTransferQueue

src/java.base/share/classes/java/util/concurrent/SynchronousQueue.java line 136:

> 134:      *     internal documentation.
> 135:      *  2. SynchronousQueues must block threads waiting to become
> 136:      *     fulfilled, preceded by brief spins at/mear front positions

Suggestion:

     *     fulfilled, preceded by brief spins at/near front positions

src/java.base/share/classes/java/util/concurrent/SynchronousQueue.java line 148:

> 146:     /**
> 147:      * Extension of LinkedTransferQueue to support Lifo (stack) mode.
> 148:      * Methods use tha "tail" field as top of stack (versus tail of

Suggestion:

     * Methods use the "tail" field as top of stack (versus tail of

src/java.base/share/classes/java/util/concurrent/SynchronousQueue.java line 149:

> 147:      * Extension of LinkedTransferQueue to support Lifo (stack) mode.
> 148:      * Methods use tha "tail" field as top of stack (versus tail of
> 149:      * queue). Note that popped nodes are not self-linked because thay

Suggestion:

     * queue). Note that popped nodes are not self-linked because they

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1250693753
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1250699320
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1255848871
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1251121355
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1250629998
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1250630384
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1250637093
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1250637972