Hi Tagir,
It’s certainly easier to review if one avoids the temptation to change other
things at the same time :-)
There is a possible simplified approach to sharing one Spliterator impl between
ArrayList and ArrayList.SubList. The initial (origin, fence) for ArrayList can
be (0, -1) and ArrayList.SubList it can be (offset, -1), as before, then one
can make it static passing in an instance of AbstractList:
if ((hi = fence) < 0) {
expectedModCount = lst.modCount; // shared field on AbstractList
hi = fence = index + lst.size(); // virtual call :-(
}
Unfortunately that requires a virtual call to lst.size(). I don’t know if we
could improve matters using a private interface contract to initialise the
state e.g.:
if ((hi = fence) < 0) {
hi = initializer.init(this); // only two possible impls of initializer
}
However, since ArrayList is quite a fundamental class my inclination is to
provide a separate implementation, as SubList already does for ListIterator.
Then we don’t perturb the optimal case, extra fields with branching or
otherwise.
Paul.
> On 4 Feb 2016, at 16:55, Tagir F. Valeev <amae...@gmail.com> wrote:
>
> Hello!
>
> Thank you for clarification. As far as I know currently JIT-compiled
> null checks are almost always implicit (via trapped page-fault). Well,
> ok, I'm not a Hotspot expert (probably for C1 this still matters). I
> will keep this code as is.
>
> Webrev was not changed:
> http://cr.openjdk.java.net/~tvaleev/webrev/8148748/r1/
> Any more reviews, please?
>
> With best regards,
> Tagir Valeev.
>
> MB> On Wed, Feb 3, 2016 at 4:20 AM, Tagir F. Valeev <amae...@gmail.com> wrote:
>
>>> Some more thoughts about forEachRemaining:
>>>
>>> To me it seems that
>>> if ((a = lst.elementData) != null)
>>> is a redundant check as well as in current ArrayList implementation
>>> elementData is never null. So it can be replaced with simple
>>> assignment.
>
> MB> The null check for something that is provably non-null is a sign of
> MB> the hand of Doug Lea.
> MB> I'm often tempted to try to remove them from j.u.concurrent as well,
> MB> but I resist.
> MB> They may help hotspot generate better code.
>
