RFR: jsr166 jdk integration 2018-05

[Stuart Marks]

> (TL;DR - replaceAll incrementing modCount is a bug.)

I acknowledge that statement is the one in dispute.

> Hmmm ... my previous convincing arguments have failed to convince ?!
> 
> Your argument above applies to List.set just as much as List.repladeAll, because 
> the latter is nothing more semantically than a bunch of calls to the former.  
> They should have the same behavior.  Not having the same behavior leads to 
> inconsistency, seen today in subList operations on ArrayList and Vector having 
> different modCount behavior than on the root list.

Right, I read those arguments, and I'm not convinced.

Just because an individual operation has some characteristic doesn't necessarily 
imply that an aggregate operation must have the same characteristic. (This is 
variously called a fallacy of composition, or in U.S. tax law, the step 
transaction doctrine.)

Bringing replaceAll() to the public API exposes it as a single operation, with 
its own semantics. Note that Vector.replaceAll() holds the lock for the entire 
operation, not merely around individual set() operations. It really is different 
from performing individual set() operations.

> Again, imagine this use case: there is a periodic background task that optimizes 
> all the elements of a Vector
> vector.replaceAll(x -> optimized(x))
> That should not break any iterations in progress.

I don't think it's possible to do that correctly without holding a lock around 
the entire iteration. If the lock is held, CME can't occur, as a concurrent 
replaceAll() will occur before or after the iteration, never in the middle.

If an iteration over a Vector doesn't hold a lock, any read-modify-write 
operations (consider a loop with a ListIterator on which set() is called) can be 
interleaved with bulk operations (like replaceAll) which is clearly incorrect. 
In such cases, CME should be thrown.

Also, this use case cannot be written today, because CME is thrown. I'm not 
aware of an actual use case that's been prevented because CME is thrown. Of 
course, as API designers we have to anticipate needs and not just react to 
complaints. My view of this kind of situation (interleaving of bulk operation(s) 
with an iteration loop) is much more likely to be a programming error than an 
actual use case.

> To strengthen that, the default method List.replaceAll is specified to be equivalent to 
> 
>      final ListIterator<E> li = list.listIterator();
>      while (li.hasNext()) {
>          li.set(operator.apply(li.next()));
>      }
> 
> and incrementing modCount breaks "equivalent to".

Note carefully: that is an "Implementation Requirement" on the default 
implementation of List.replaceAll(). It doesn't govern implementations in 
implementing classes such as ArrayList.

s'marks