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

Сергей Цыпанов sergei.tsypanov at yandex.ru
Tue Jun 11 11:24:12 UTC 2019


Hello Tagir,

you did a great job about those methods!

I've got a question regarding variables 'r' in lines 946 and 969:

why do you need them when 'a' is returned from the method?

Regards,
Sergey Tsypanov

11.06.2019, 05:50, "Tagir Valeev" <amaembo at gmail.com>:
> 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
>
>>


More information about the core-libs-dev mailing list