RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

[Claes Redestad]

On 2018-05-14 22:29, Ivan Gerasimov wrote:
> Thank you Claes!
>
> The mutator methods normally first update the modCount and then change 
> the size of ArrayList.
>
> Then, in other methods the modCount is copied to a local variable 
> first, and after that the size is copied.
>
> This is not true for equalsRange(List<?> other, int from, int to) when 
> it is called from ArrayList.equals: the size is first copied to the 
> argument and then the modCount is checked inside of equalsRange().  If 
> the size and modCount are changed in between, then equals may produce 
> a wrong results.
>
> It seems to be more accurate to store this.modCount prior calling to 
> equalsRange((List<?>) o, 0, size); and do 
> checkForComodification(expectedModCount); after it is done.
>
> Checking for modCount inside equalsRange() can probably be safely 
> removed.
>
> There's another call to equalsRange() from SubList.equals().  In this 
> case checkForComodification(); is already called after calling to 
> equalsRange(), so everything seems to be fine here.

I was actually toying with and testing a change to this effect anyway, 
since it's a nice cleanup and might help the JIT somewhat:

http://cr.openjdk.java.net/~redestad/8196340/open.02/

A note on correctness: none of the code in ArrayList that utilize 
modCount is "correct" or accurate in a concurrency or thread-safety 
context where ordering of loads and stores is critical for correctness: 
lacking volatile or other means to ensure correct ordering of operations 
(or atomicity of modCount increments) the runtime is given large amounts 
of freedoms to do as it please to optimize things.  This is intentional 
as ArrayList isn't supposed to be neither thread-safe nor concurrent on 
its own.  The main utility is instead about detecting and avoid coding 
errors, mainly bugs like adding to or removing items from a list you're 
simultaneously iterating over. So what I'm saying is that the ordering 
of operations within a method might not be all that important as long as 
they are sound within the scope of the method execution. Still, making 
it a bit more like correct concurrent code might improve the best effort 
behavior in some circumstances, and costs us nothing.

Thanks - and sorry for dragging you along for a bit longer...

/Claes