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

Reply via email to