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