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.ma...@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 >
