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

XenoAmess duke at openjdk.java.net
Sat Mar 12 01:35:28 UTC 2022


On Fri, 11 Mar 2022 19:09:25 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> XenoAmess has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - change KeyStructure to String
>>  - fix test
>
> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 61:
> 
>> 59:         String String = Integer.toString(i);
>> 60:         map.put(String, String);
>> 61:     }
> 
> Small details here... the `String` variable should be lower case. But really this method should be inlined into `putN`, since that's the method that needs to generate mappings. Then, `makeMap` should call `putN`.

@stuart-marks done.

> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 107:
> 
>> 105:         for (int i = 0; i < size; ++i) {
>> 106:             putMap(map, i);
>> 107:         }
> 
> Replace this loop with a call to `putN`.

@stuart-marks done.

> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 131:
> 
>> 129:     void putN(Map<String, String> map, int n) {
>> 130:         for (int i = 0; i < n; i++) {
>> 131:             putMap(map, i);
> 
> Inline `putMap` here.

@stuart-marks done.

> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 317:
> 
>> 315:     public void defaultCapacity(Supplier<Map<String, String>> s) {
>> 316:         Map<String, String> map = s.get();
>> 317:         putMap(map, 0);
> 
> `map.put("", "")`

@stuart-marks done.

> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 343:
> 
>> 341:     public void requestedCapacity(String label, int cap, Supplier<Map<String, String>> s) {
>> 342:         Map<String, String> map = s.get();
>> 343:         putMap(map, 0);
> 
> No need to generate a key/value pair here, just use string literals, e.g. `map.put("", "")`.

@stuart-marks done.

> test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 430:
> 
>> 428:                     map.putAll(makeMap(size));
>> 429:                 })
>> 430:         );
> 
> Wait, did this get reindented? Adding line breaks totally destroys the tabular nature of test data. Please restore. Yes, the lines end up being longer than the usual limit, but the benefit of having things in a nice table outweighs to cost of having the lines be somewhat over-limit.

@stuart-marks done.

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

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


More information about the core-libs-dev mailing list