RFR-8148748: ArrayList.subList().spliterator() is not late-binding

[Paul Sandoz]

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 <amaembo at 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 <amaembo at 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.
>