RFR: jsr166 jdk integration 2018-05

[Paul Sandoz]

> On May 14, 2018, at 2:04 PM, Martin Buchholz <martinrb at google.com> wrote:
> 
> 
> 
> On Mon, May 14, 2018 at 1:45 PM, Paul Sandoz <paul.sandoz at oracle.com <mailto:paul.sandoz at oracle.com>> wrote:
> 
> 
>> On May 14, 2018, at 12:43 PM, Martin Buchholz <martinrb at google.com <mailto:martinrb at google.com>> wrote:
>> 
>> 
>> 
>> On Mon, May 14, 2018 at 12:18 PM, Paul Sandoz <Paul.Sandoz at oracle.com <mailto:Paul.Sandoz at oracle.com>> wrote:
>>> 
>>> A CME is not necessarily associated with just structural modifications it could, on a best effort basis, be associated with any modification, which is cheaper to do for bulk operations rather than individual operations, and this operation can be used to perturb all the elements of the list (just like sort) causing strange and hard to track bugs while in the middle of iterating. IMHO its better to fail under such circumstances rather than be silent.
>>> 
>>> It's tempting to treat modCount as a count of ALL modifications, especially given its name, but that's different from the historical behavior and design of these classes.  Consistency with existing spec and implementations is the biggest argument. 
>> 
>> I mentally revised the history when doing the collections/stream API work since we added more bulk operations, since this is on a “best effort” basis and if it’s cheap to do then there is no real harm in it and it might help.
>> 
>> 
>> Spec says:
>> 
>> """protected transient int modCount
>> 
>> The number of times this list has been structurally modified. Structural modifications are those that change the size of the list, or otherwise perturb it in such a fashion that iterations in progress may yield incorrect results."""
>> 
>> replaceAll doesn't qualify as a structural modification. 
> 
> Why not? It can "perturb it in such a fashion that iterations in progress may yield incorrect results”.
> 
> Why?  It replaces every element "inplace" in the style of List.set(i) which is also not a structural modification.

I get that, but I would argue that (placing the implementation aside) the bulk operation as a whole is compatible with the “otherwise” part of the modCount definition, since it can perturb the list and effect the yet to be traversed elements. Such usage is a likely source of hard to track down bugs that CMEs are designed to help flag.

I doubt i am gonna change your mind on this :-) So we may just have to agree to disagree on the interpretation of the definition and move on. I would prefer it remains but it's your call.

Paul.

> For ArrayList and Vector in particular, List.replaceAll is as safe as List.set(int).
> And if there was a List implementation for which that was not the case, it would probably be a bug in replaceAll implementation.
> 
> Think ordinary array assignment or AtomicReferenceArray assignment.