RFR: 8316493: Make immutable maps @ValueBased

[Per Minborg]

On Thu, 7 Sep 2023 11:13:44 GMT, Per Minborg <pminborg at openjdk.org> wrote:

> This PR outlines a solution for making immutable maps `@ValueBased` by removing cacheing of certain values in `AbstractMap`.
> 
> By removing these caching fields in `AbstractMap`, we can make the immutable maps `@ValueBased` and at the same time, performance is likely improved because the JVM is probably able to optimize away object creation anyway via escape analysis. Also, all maps will occupy less space as we get rid of a number of objects and references stored for each map.
> 
> We need to benchmark this solution to better understand its implications.

@cl4es ran some benchmarks:


Name                      (size) Cnt  Base   Error   Test   Error   Unit   Diff%
HashMapViews.entrySetSize      1   5 1,085 ± 0,118  1,003 ± 0,053  ns/op    7,6% (p = 0,002*)
  :gc.alloc.rate             N/A   5 0,013 ± 0,000  0,013 ± 0,000 MB/sec   -0,1% (p = 0,639 )
  :gc.alloc.rate.norm        N/A   5 0,000 ± 0,000  0,000 ± 0,000   B/op   -7,7% (p = 0,003*)
  :gc.count                  N/A   5 0,000          0,000         counts
HashMapViews.entrySetSize   1000   5 1,001 ± 0,026  1,033 ± 0,170  ns/op   -3,3% (p = 0,173 )
  :gc.alloc.rate             N/A   5 0,013 ± 0,000  0,013 ± 0,000 MB/sec    0,6% (p = 0,012 )
  :gc.alloc.rate.norm        N/A   5 0,000 ± 0,000  0,000 ± 0,000   B/op    3,9% (p = 0,124 )
  :gc.count                  N/A   5 0,000          0,000         counts
HashMapViews.keySetSize        1   5 1,051 ± 0,017  0,706 ± 0,008  ns/op   32,8% (p = 0,000*)
  :gc.alloc.rate             N/A   5 0,013 ± 0,000  0,013 ± 0,000 MB/sec    0,0% (p = 0,947 )
  :gc.alloc.rate.norm        N/A   5 0,000 ± 0,000  0,000 ± 0,000   B/op  -32,8% (p = 0,000*)
  :gc.count                  N/A   5 0,000          0,000         counts
HashMapViews.keySetSize     1000   5 1,078 ± 0,013  0,705 ± 0,006  ns/op   34,6% (p = 0,000*)
  :gc.alloc.rate             N/A   5 0,013 ± 0,000  0,013 ± 0,000 MB/sec   -0,1% (p = 0,756 )
  :gc.alloc.rate.norm        N/A   5 0,000 ± 0,000  0,000 ± 0,000   B/op  -34,7% (p = 0,000*)
  :gc.count                  N/A   5 0,000          0,000         counts
HashMapViews.valuesSize        1   5 0,998 ± 0,033  0,718 ± 0,062  ns/op   28,1% (p = 0,000*)
  :gc.alloc.rate             N/A   5 0,013 ± 0,000  0,013 ± 0,000 MB/sec    0,7% (p = 0,055 )
  :gc.alloc.rate.norm        N/A   5 0,000 ± 0,000  0,000 ± 0,000   B/op  -27,6% (p = 0,000*)
  :gc.count                  N/A   5 0,000          0,000         counts
HashMapViews.valuesSize     1000   5 1,043 ± 0,062  0,705 ± 0,011  ns/op   32,4% (p = 0,000*)
  :gc.alloc.rate             N/A   5 0,013 ± 0,000  0,013 ± 0,000 MB/sec   -0,3% (p = 0,405 )
  :gc.alloc.rate.norm        N/A   5 0,000 ± 0,000  0,000 ± 0,000   B/op  -32,6% (p = 0,000*)
  :gc.count                  N/A   5 0,000          0,000         counts
  * = significant

Invariant parameters used by above microbenchmarks:
mapType:   HashMap

src/java.base/share/classes/java/util/AbstractMap.java line 323:

> 321:         return new AbstractSet<>() {
> 322:             public Iterator<K> iterator() {
> 323:                 return new Iterator<>() {

There is another PR that proposes to refactor these anonymous classes: https://github.com/openjdk/jdk/pull/15615/files

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

PR Comment: https://git.openjdk.org/jdk/pull/15614#issuecomment-1710165103
PR Review Comment: https://git.openjdk.org/jdk/pull/15614#discussion_r1329664799