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

Stuart Marks stuart.marks at oracle.com
Thu Jun 6 19:43:39 UTC 2019


On 6/5/19 9:25 PM, Tagir Valeev wrote:
> Thanks for review. Indeed, you have a point about behavioral change. I filed a 
> CSR:
> https://bugs.openjdk.java.net/browse/JDK-8225393
Great, I've made some edits to the CSR and I've added myself as a reviewer. It 
should be ready for you to move into the Finalized state. Please wait for the 
CSR group (probably Joe Darcy) to mark it Approved before pushing this changeset.
> I wanted to generate a specdiff and attach to CSR, but it seems that I don't 
> know how to do it :(
> I found nothing in OpenJDK Wiki about this. Could somebody help me?

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.)

> 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.

Oh, one more thing in LinkedHashMap.java:

565 public Object[] toArray() { return keysToArray(new Object[size]); }

Please add line breaks.

Thanks,

s'marks




More information about the core-libs-dev mailing list