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

[Doug Lea]

On Fri, 21 Jul 2023 09:30:53 GMT, Viktor Klang <duke at openjdk.org> wrote:

>> Doug Lea has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   nitpicks
>
> src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 397:
> 
>> 395:          * True if system is a uniprocessor, occasionally rechecked.
>> 396:          */
>> 397:         private static boolean isUniprocessor =
> 
> @DougLea  If this is intentionally non-volatile, I think it is worth documenting the rationale.

Added: (L348)
     * than unnecessarily requiring volatile accesses elsewhere. This
     * fence also separates accesses to field isUniprocessor.

> src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 446:
> 
>> 444:                         Thread.onSpinWait();
>> 445:                     else
>> 446:                         LockSupport.parkNanos(ns);
> 
> @DougLea If `ns` is sufficiently large, would it not make sense to use managed blocking in case the current thread is a FJWT as below? 🤔

It's a good point, but we don't normally do this. Added (L336):
     * returns just barely too soon. As is the case in most j.u.c
     * blocking support, untimed waits use ManagedBlockers when
     * callers are ForkJoin threads, but timed waits use plain
     * parkNanos, under the rationale that known-to-be transient
     * blocking doesn't require compensation. (This decision should be
     * revisited here and elsewhere to deal with very long timeouts.)

> src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java line 612:
> 
>> 610:         else if (m != null)
>> 611:             s.selfLinkItem();
>> 612:         return m;
> 
> @DougLea I'd probably add a newline before the return statement to visually distinguish that it isn't intended to be read as a part of the if-else branches.

Thanks. Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270541557
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270543495
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1270543842