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

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