RFR: 8215995: Add specialized toArray methods to immutable collections

[Claes Redestad]

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? 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

> 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. :-)

/Claes