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