RFR: 8166365: Small immutable collections should provide optimized implementations when possible

[Stuart Marks]

On 1/11/17 5:43 AM, Claes Redestad wrote:
>> List2.hashCode()
>> ListN.hashCode()
>> MapN.hashCode()
>>
>> aren't overridden.
>>
>> I'd like to see them added at some point, but if your benchmarks don't justify
>> them, maybe they're not necessary right now. If not, we should make a note to
>> add them later.
>
> As they are all rather simple and straightforward I've added them for consistency.

OK, thanks for adding these.

>> In Set2, SetN, Map1, and MapN, the addition of @Stable also dropped the
>> "private" access modifier. Other implementations still have corresponding
>> fields as private. Was this intentional? I think they should all remain private.
>
> This was intentional: for those implementations where I dropped private there
> are inner
> classes which access the fields directly.  This leads to synthetic accessors,
> which shows up
> as JIT compilation overhead during module bootstrap profiling.
>
> This is what made me look at this in the first place, and a large part of the
> reason why I
> believe that it's motivated to fast-track this enhancement as a means to improve
> JDK 9
> startup metrics.  If you have a strong preference against them being made

[snipped example of using static classes]

Oh, right! I was aware of the issue of the accessor methods, but somehow I 
forgot about it when reviewing the code.

For the implementations that have inner classes (Set2, SetN, and MapN) I think 
it's fine for the fields to have package access instead of being private. It 
doesn't seem worth it to do the refactoring of the inner classes to static 
classes, just to make the fields private. (In fact, I seem to recall that an 
earlier version of the code did use static classes, and I had refactored it to 
use inner classes, reducing the clutter considerably.)

But note, it looks like Map1 dropped "private" even though it doesn't have any 
inner classes. So maybe just restore "private" for Map1's fields.

> All done:
>
> http://cr.openjdk.java.net/~redestad/8166365/webrev.02/

Looks great! If all you do is restore private fields in Map1, no need for 
another webrev.

Thanks,

s'marks