RFR: 8215995: Add specialized toArray methods to immutable collections

[Martin Buchholz]

I'm happy with what you've got now.

On Wed, Jan 2, 2019 at 11:46 AM Claes Redestad <claes.redestad at oracle.com>
wrote:

> Hi,
>
> On 2019-01-02 20:26, Martin Buchholz wrote:
> >
> >
> > On Wed, Jan 2, 2019 at 11:10 AM Claes Redestad
> > <claes.redestad at oracle.com <mailto:claes.redestad at oracle.com>> wrote:
> >
> >
> >      > I was surprised by the use of SALT in SetN. I'm guessing you could
> >      > improve SetN by adapting the tricky circular array traversal code
> >     from
> >      > ArrayDeque.
> >      > I'm not convinced that the extra non-determinism from negative
> SALT
> >      > pulls its weight.
> >
> >     This is one of Stuart's designs - the goal being to avoid
> >     situations where tests and production code create an unintended
> >     dependency on the iteration order - giving us more freedom to change
> >     hash algorithm etc. Since the SALT is calculated only once the
> untaken
> >     paths should be DCE'd, so we're not paying any real cost for this.
> >
> >
> > you could elide the creation of an Iterator object in toArray.
>
> I'm assuming you mean SetN::toArray, and yes I could, but that'd require
> replicating most of the logic in SetNIterator which didn't seem
> worthwhile to get rid of an iterator allocation. I can run the numbers:
> likely a quite small improvement, which I'm not sure will be worth the
> extra duplication.
>
> > you could elide the bounds check on the second leg of the iteration if
> > you rewrote the loop in ArrayDeque style.
>
> Which bounds check?


On the second leg, you know you'll never hit the end of the array, so you
can elide the check.  But hotspot may not be so clever and may re-insert
it, in a slightly more expensive variant that has actual exception throwing
code.


> If this is about the if (SALT >= 0)..else-construct
> in SetNIterator::nextIndex then the JIT should ideally only care about
> the live branch (since SALT doesn't mutate the other leg is effectively
> dead). This matches my reading of the JITted code as given by running
> micros with -prof perfasm
>
> I never added perfasm to my toolbox.  Maybe next year.  Oh wait, it's
already next year ...


> > But on modern hardware with a modern JIT you might not even notice.
>
> That's definitely a possibility.  I do try look at behavior in the
> interpreter and -XX:TierStopAtLevel=1 as a gauge for how things behave
> in simpler worlds. :-)
>
>
I do something similar with my less sophisticated benchmark.


> /Claes
>