RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v28]

[Stuart Marks]

On Thu, 10 Mar 2022 16:10:31 GMT, XenoAmess <duke at openjdk.java.net> wrote:

>> XenoAmess has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - clean out tests
>>  - Remove 'randomness' keyword.
>>  - Cleanup and commenting.
>>  - initial rewrite of WhiteBoxResizeTest
>>  - Restore WhiteBoxResizeTest.java
>
> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 290:
> 
>> 288:         cases.addAll(genPopulatedCapacityCases(12,  16));
>> 289:         cases.addAll(genPopulatedCapacityCases(13,  32));
>> 290:         cases.addAll(genPopulatedCapacityCases(64, 128));
> 
> @stuart-marks should there better be a loop from 1 to 64?

The difficulty with having a loop instead of constants is that the expected value now needs to be computed. We could probably use `tableSizeFor` to do this. But this is starting to get uncomfortably close to a testing antipattern which is to use the code under test as part of the test. If a bug is introduced into `tableSizeFor`, it could introduce the error into both the actual value and the expected value, covering up the bug. (This is related to one of the flaws with the Enum/ConstantDirectoryOptimalCapacity test.) Now we _hope_ that `tableSizeFor` is correct and tested, but that verges on having tests depend on each other in a subtle and uncomfortable way.

Recall that one of the original cases was that the table size computation

    (int) (numMappings / 0.75f) + 1

gives a value one too large: for 12 mappings it results in 17, giving a table size of 32. The (12, 16) case is sufficient to verify that this is fixed. Then the adjacent cases are there to make sure nothing weird is going on. (You could argue that (11, 16) isn't necessary.)

I threw in the (64, 128) case because of the loop to 64, but it's not necessarily testing anything useful. If you think we need more cases we can add more at the boundaries, such as (24, 32) and (25, 64).

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

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