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

Martin Buchholz martinrb at google.com
Fri Dec 8 17:44:39 UTC 2017


Should there be changes to general purpose collection testing classes like
test/jdk/java/util/concurrent/tck/CollectionTest.java
test/jdk/java/util/Collection/MOAT.java
that would have caught this bug?
(although those are mostly designed for mutable collections, but I think
some immutable collections (nCopies?) get tested somewhere.

On Fri, Dec 8, 2017 at 7:13 AM, Claes Redestad <claes.redestad at oracle.com>
wrote:

> Hi,
>
> On 2017-12-08 07:54, Andrej Golovnin wrote:
>
>> Hi Claes,
>>
>> http://cr.openjdk.java.net/~redestad/8193128/open.02/
>>>
>> I think you should provide specialised implementations of the
>> #indexOf() and #lastIndexOf() methods in the classes List12 and ListN.
>> Using an iterator in List12 is an overkill. But even in ListN using a
>> simple for loop would be much better.
>>
>
> it's somewhat ironic that I started looking at this *because*
> indexOf was slow due use of iterators[1], but then never got
> around to specialize them in this patch.
>
> In any case please take look at
>> the implementation of the #lastIndexOf() method in the
>> AbstractImmutableList class. It looks like a copy of
>> AbstractImmutableList#indexOf() and this is wrong.
>>
>
> Nice catch! Quite the embarrassing copy-paste that...
>
> - Specialized the index/lastIndexOf methods for List12, ListN
> - Fixed implementation of lastIndexOf in AbstractImmutableList.
> - As AbstractImmutableList.indexOf/lastIndexOf are now only used
> by AbstractImmutableList.SubList, I moved them there with some
> commentary since it's not clear JDK-8191418 should add null
> checkson the input for SubList or not.
> - Added sanity tests of indexOf/lastIndexOf that touches all
> the new implementations:
>
> http://cr.openjdk.java.net/~redestad/8193128/open.03/
>
> Thanks!
>
> /Claes
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8191442
>


More information about the core-libs-dev mailing list