On Thu, 16 Feb 2023 19:44:35 GMT, Viktor Klang <d...@openjdk.org> wrote:

>> src/java.base/share/classes/java/util/stream/ForEachOps.java line 431:
>> 
>>> 429:                 // leftChild and rightChild were just created and not 
>>> fork():ed
>>> 430:                 // yet so no need for a volatile write
>>> 431:                 NEXT.set(leftChild, rightChild);
>> 
>> Make `next` a plain field and shuffle up the assignment (`leftChild.next = 
>> rightChild`) to occur immediately after construction of the right child 
>> task? (FWIW `addToPendingCount` operates on a volatile field of 
>> `CountedCompleter`).
>
> @PaulSandoz I'm usually a bit weary of piggybacking if it is not done on the 
> same object, as future reorderings of the code might break that assumption. I 
> wouldn't want to break anything silently so I made a rather conservative 
> change. On which side should I err? :)

Since as you have said the left and right nodes have yet to be "published" 
(outside of their little nest) I think it should be fine to move it above and 
next to the constructions.
(Also since `addToPendingCount` has volatile write semantics, via it's 
getAndSet, the plain write should not float below that call if that really 
matters, and its hard to see the code radically changing in this regard unless 
there is a major rewrite than i guess it all has to be carefully rethought.)

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

PR: https://git.openjdk.org/jdk/pull/12320

Reply via email to