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

Tagir Valeev amaembo at gmail.com
Tue Jun 11 03:45:51 UTC 2019


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