On Fri, 21 Jul 2023 09:30:53 GMT, Viktor Klang <[email protected]> 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
