On Fri, 10 Nov 2023 08:17:22 GMT, Per Minborg <pminb...@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. > > Per Minborg has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 14 commits: > > - Merge branch 'master' into vb-map2 > - Fix formatting > - Remove caching in TreeMap > - Remove caching from CHM and CSLM > - Move back clone to original position > - Reintroduce AbstractMap::clone > - Add 'fresh' to implSpec > - Remove AbstractMap::clone > - Merge master > - Merge branch 'master' into vb-map2 > - ... and 4 more: https://git.openjdk.org/jdk/compare/9cce9fe0...b1bfcd17 src/java.base/share/classes/java/util/AbstractMap.java line 322: > 320: public boolean isEmpty() { return > AbstractMap.this.isEmpty(); } > 321: public void clear() { AbstractMap.this.clear(); } > 322: public boolean contains(Object k) { return > AbstractMap.this.containsKey(k); } Suggestion: public boolean contains(Object k) { return containsKey(k); } Maybe we should have consistency of style. If we qualify here, we should also qualify `AbstractMap.this.new KeyIterator()` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15614#discussion_r1633556698
