RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

[Tagir Valeev]

Hello!

Thanks, pushed:
http://hg.openjdk.java.net/jdk/jdk/rev/1afe0cb93482

On Fri, Jun 14, 2019 at 11:28 AM Stuart Marks <stuart.marks at oracle.com>
wrote:

> Hi Tagir,
>
> I reviewed your latest changeset and it looks fine.
>
> I ran the changes through our internal test system and the results look
> good.
>
> Since the CSR was approved for JDK 14, and the mainline is now JDK 14
> (after the JDK 13 RDP1 fork), you can now push this changeset.
>
> Thanks,
>
> s'marks
> On 6/10/19 8:45 PM, Tagir Valeev wrote:
>
> Hello!
>
> I addressed all your suggestions, please check the updated webrev
> (targeted for JDK 14):
> http://cr.openjdk.java.net/~tvaleev/webrev/8225339/r3/
>
> As you noted elsewhere, the way the build runs javadoc shouldn't generate
>> any specification differences, so we don't need to worry about specdiff.
>> I've applied your patch to my personal tree and I've verified that there
>> aren't any specification differences. (Indeed, the only difference is that
>> the HTML files have an additional meta tag; I'm not sure of the
>> significance of this, but it doesn't seem to impact the specification as
>> far as I can see.)
>>
>
> Thank you for doing this for me!
>
>
>> Code-related suggestions are addressed in new webrev (along with Roger's
>> suggestions):
>> http://cr.openjdk.java.net/~tvaleev/webrev/8225339/r2/
>>
>> Small item: HashMap.values() and LinkedHashMap.values() still have the
>> issue where the no-arg toArray() allocates a right-sized array and then
>> calls toArray(T[]), which then calls prepareArray(), which rechecks the
>> size. Thanks for changing the other places, but I think these should be
>> changed too. Unfortunately I think this means introducing a helper method
>> for values, similar to keysToArray(), and then both toArray() overloads can
>> call it. But I think this makes good sense and aligns the implementation
>> with the other toArray() implementations.
>>
> I tried to make patch smaller and did not extract valuesToArray because
> there's no need to call it externally (keysToArray should be called from
> HashSet). But you're right, making the code more uniform looks better than
> making the code smaller.
>
>
>> Oh, one more thing in LinkedHashMap.java:
>>
>>  565         public Object[] toArray() { return keysToArray(new Object[size]); }
>>
>> Please add line breaks.
>>
> Oops, missed that. Thanks!
>
> With best regards,
> Tagir Valeev
>
>>