[11] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[Zheka Kozlov]

I noticed that List.copyOf() allocates an array twice. The first allocation
is coll.toArray(), the second one is in ListN constructor. Can we do
something with it?

2018-03-22 7:51 GMT+07:00 Stuart Marks <stuart.marks at oracle.com>:

> Hi Claes,
>
> I'm finally finally getting back to this. I reviewed the current state of
> the patch located here:
>
>     http://cr.openjdk.java.net/~redestad/8193128/open.06/
>
> I think this is mostly fine as it is. There are some things about it that
> could be adjusted, but none of them stand in the way of it going in. If you
> want to take care of any of these items before pushing, that'd be great,
> otherwise they can be handled separately later.
>
> * AbstractImmutableCollection and AbstractImmutableSet are in the "List
> Implementations" section of the file. Seems like AIC ought to be moved to
> the top, since it's common to List and Set, and AIS ought to be moved to
> the "Set Implementations" section. This is just location in the file, not
> nesting level.
>
> * The SubList constructors that are overloaded based on the type of the
> 1st arg (List vs SubList) seems subtle and error-prone. I misread the code
> the first time I saw it. Seems like it would be preferable to have
> well-named static factory methods, each calling a policy-free (private)
> constructor.
>
> * Should SubList.size be final? Should any of the SubList fields be
> @Stable?
>
> * Does the SubList class need to nested within AbstractImmutableList? Note
> that it also extends AIL, so I don't think nesting gives it access to
> anything that it doesn't already have. Plus it includes an anonymous inner
> class based on ListIterator, which is a fourth level of nested classes.
> It'd be good to flatten this out a bit if possible.
>
> * The instance returned by SubList.iterator() is also a ListIterator.
> Hmmm. But I'm also wondering if SubList's iterators can be shared somehow
> with AIL's Itr and ListItr.
>
> * Several indexOf tests are added to ListFactories.java. Are these
> redundant with the indexOf tests that were added to MOAT?
>
> Let me know what, if any, you fix up before pushing, and I'll track the
> rest.
>
> Thanks,
>
> s'marks
>