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

Claes Redestad claes.redestad at oracle.com
Mon May 14 14:33:24 UTC 2018


Martin,

On 2018-05-14 16:15, Martin Buchholz wrote:
> Claes,
>
> Looks good.

thanks!

>
> I would move the size check up to the beginning of the method.
>
> 583         int expectedModCount = modCount;
>  584         int otherModCount = other.modCount;
>  585         int s = size;
>  586         if (s != other.size) {
>  587             return false;
>  588         }

Sure.

>
> I would prefer having only one comodification check for a bulk 
> operation, but I understand that checking at each step is more 
> compatible with the default implementation.
>
>  594         for (int i = 0; i < s; i++) {
>  595             if (!Objects.equals(es[i], otherEs[i])) {
>  596  other.checkForComodification(otherModCount);
>  597  checkForComodification(expectedModCount);
>  598                 return false;
>  599             }
>  600         }
>

Perhaps you misread as I think the patch does what I think you're 
suggesting and only checks once. True, this is subtly
different in behavior from the current implementation
which is fail-fast.

While it shouldn't matter semantically, it could in theory mean a 
measurable delay in throwing a CME when dealing
with large lists.  As CMEs in ArrayLists could be considered a coding 
error - and the speedup of only checking once
for a bulk operation like equal is significant - I think it's a 
reasonable trade-off to go ahead with this.

/Claes


More information about the core-libs-dev mailing list