RFR: 8186958: Need method to create pre-sized HashMap [v14]

Stuart Marks smarks at openjdk.java.net
Wed Apr 13 04:28:22 UTC 2022


On Sun, 10 Apr 2022 20:28:16 GMT, XenoAmess <duke at openjdk.java.net> wrote:

>> 8186958: Need method to create pre-sized HashMap
>
> XenoAmess has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits:
> 
>  - Merge branch 'master' into fix_8186958
>  - variable nameToReferenceSize rename to moduleCount
>  - use (double) DEFAULT_LOAD_FACTOR instead of 0.75
>  - drop CalculateHashMapCapacityTestJMH
>  - refine javadoc for LinkedHashMap#newLinkedHashMap
>  - revert changes in jdk.compile
>  - update usages of LinkedHashMap
>  - update usages of HashMap
>  - update codes
>  - update jmh
>  - ... and 7 more: https://git.openjdk.java.net/jdk/compare/34914f12...7adc89c1

OK, the CSR is approved now.

Regarding the load factor of 0.75, I'm not quite sure whether there is a rigorous justification for this value. It seems "traditional." It does seem likely that using a load factor of 0.75 will generate fewer collisions than a load factor of 1.0. It would be an interesting for a future investigation to see how much of a performance difference the load factor makes. In practice I think there are very few cases where it makes sense to use anything other than the default. In any case, there's no reason to change anything from the current default of 0.75.

It occurs to me that `HashSet` and `LinkedHashSet` have the same "capacity" problem. It would be good to add static factory methods for them too. This is probably best done as a separate effort: see [JDK-8284780](https://bugs.openjdk.java.net/browse/JDK-8284780).

I've done some work on add test cases for these new static factory methods, and I've also added API notes to the capacity-based constructors to link to the new factory methods. Note that even though these are javadoc changes, the API notes are non-normative and don't require an update to the CSR. See the last few commits on this branch:

https://github.com/stuart-marks/jdk/commits/JDK-8186958-presized-HashMap

If you think they're good, please pull them in.

Regarding the scope of changes, the number of call sites that are changed is indeed spread rather too widely across the JDK. In order to keep the number of required reviewers small, I think we should trim down the call sites to a more manageable set. Specifically, I'd suggest **removing** from this PR the changes from the files in the following areas:

 * src/java.desktop
 * src/java.management
 * src/jdk.internal.vm.ci
 * src/jdk.jfr
 * src/jdk.management.jfr
 * src/jdk.management
 * src/utils/IdealGraphVisualizer

After removing these, there will still be a fair number of call sites. Several of these have errors, so they'll be sufficient to show the value of the new APIs. After that I'll recompute and readjust labels to pull in the right set of reviewers.

Thanks.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7928


More information about the core-libs-dev mailing list