JDK-8210280 - Unnecessary reallocation when invoking HashMap.putAll()

[Michal Vala]

I was thinking about testing resizing with items in buckets.
Test like this. Can you please include it to changeset? (I can see you have 
prepared full webrev from changeset, so I'm not posting new one.)

     @Test
     public void resizeInternalTableWithBucketsTest() {
         Map m = new HashMap();
         // Default size of HashMap table is 16. Make sure there are multiple 
items in one bucket.
         for (int i = 0; i < 4; i++) {
             m.put((i * 16) + 7, 1);
         }

         Map m2 = new HashMap();
         for (int i = 0; i < 64; i++) {
             m2.put(-i, 2);
         }

         // put bigger map to ensure multiple resizing
         m.putAll(m2);

         // all original items should be there and properly rehashed
         for (int i = 0; i < 4; i++) {
             assertEquals(m.get((i * 16) + 7), 1);
         }
     }

On 12/19/18 3:47 PM, Martin Buchholz wrote:
> Let's add whitebox tests for initial capacity and LinkedHashMap, as with
> ConcurrentHashMap's whitebox tests.
> 
> --- src/test/jtreg/util/HashMap/WhiteBoxResizeTest.java 18 Dec 2018
> 20:21:24 -0000 1.1
> +++ src/test/jtreg/util/HashMap/WhiteBoxResizeTest.java 19 Dec 2018
> 14:35:50 -0000
> @@ -27,7 +27,11 @@
>   import java.lang.invoke.MethodType;
>   import java.lang.invoke.VarHandle;
>   import java.util.HashMap;
> +import java.util.LinkedHashMap;
> +import java.util.List;
>   import java.util.Map;
> +import java.util.concurrent.ThreadLocalRandom;
> +import java.util.function.Supplier;
>   import java.util.stream.IntStream;
> 
>   import static java.util.stream.Collectors.toMap;
> @@ -42,6 +46,7 @@
>    * @run testng WhiteBoxResizeTest
>    */
>   public class WhiteBoxResizeTest {
> +    final ThreadLocalRandom rnd = ThreadLocalRandom.current();
>       final MethodHandle TABLE_SIZE_FOR;
>       final VarHandle THRESHOLD;
>       final VarHandle TABLE;
> @@ -91,14 +96,36 @@
>       }
> 
>       @Test
> -    public void capacityTest() {
> -        HashMap<Integer, Integer> map = new HashMap<>();
> +    public void capacityTestDefaultConstructor() {
> +        capacityTestDefaultConstructor(new HashMap<>());
> +        capacityTestDefaultConstructor(new LinkedHashMap<>());
> +    }
> +
> +    void capacityTestDefaultConstructor(HashMap<Integer, Integer> map) {
>           assertNull(table(map));
> 
>           map.put(1, 1);
> -        assertEquals(capacity(map), 16);
> +        assertEquals(capacity(map), 16); // default initial capacity
> 
>           map.putAll(IntStream.range(0, 64).boxed().collect(toMap(i -> i, i
> -> i)));
>           assertEquals(capacity(map), 128);
>       }
> +
> +    @Test
> +    public void capacityTestInitialCapacity() {
> +        int initialCapacity = rnd.nextInt(1, 256);
> +        List<Supplier<HashMap<Integer, Integer>>> suppliers = List.of(
> +            () -> new HashMap<>(initialCapacity),
> +            () -> new HashMap<>(initialCapacity, 0.75f),
> +            () -> new LinkedHashMap<>(initialCapacity),
> +            () -> new LinkedHashMap<>(initialCapacity, 0.75f));
> +
> +        for (Supplier<HashMap<Integer, Integer>> supplier : suppliers) {
> +            HashMap<Integer, Integer> map = supplier.get();
> +            assertNull(table(map));
> +
> +            map.put(1, 1);
> +            assertEquals(capacity(map), tableSizeFor(initialCapacity));
> +        }
> +    }
>   }
> 

-- 
Michal Vala
OpenJDK QE
Red Hat Czech