RFR: 8353273: Reduce number of oop map entries in instances [v2]
Leonid Mesnik
lmesnik at openjdk.org
Thu Apr 3 22:55:53 UTC 2025
On Wed, 2 Apr 2025 09:42:05 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> In preparation for planned GC performance improvements (KLUT), I would like to reduce the average number of oop map entries.
>>
>> For details, please see JBS issue text.
>>
>> -----------------------
>>
>> Patch results:
>>
>> The patch brings a positive change of oop map size, reducing the likelihood of lengthy oop maps. Here the oop map size distribution over all JDK classes in the JDK image:
>>
>> Before:
>>
>> 5395 - non-static oop maps (0 entries)
>> 9330 - non-static oop maps (1 entries)
>> 1449 - non-static oop maps (2 entries)
>> 274 - non-static oop maps (3 entries)
>> 218 - non-static oop maps (4 entries)
>> 75 - non-static oop maps (5 entries)
>> 7 - non-static oop maps (6 entries)
>> 4 - non-static oop maps (7 entries)
>>
>>
>> Now:
>>
>> 5395 - non-static oop maps (0 entries)
>> 10178 - non-static oop maps (1 entries)
>> 933 - non-static oop maps (2 entries)
>> 229 - non-static oop maps (3 entries)
>> 16 - non-static oop maps (4 entries)
>> 1 - non-static oop maps (5 entries)
>>
>>
>> For example, `java.util.concurrent.ConcurrentHashMap$TreeNode` is changed from having 2 entries to having just one entry, which is nice for a class that may be instantiated a lot:
>>
>> Before:
>>
>> java.util.concurrent.ConcurrentHashMap$TreeNode {0x000000000d1dddc0}
>> - ---- non-static fields (9 words):
>> - final 'hash' 'I' @12
>> - final 'key' 'Ljava/lang/Object;' @16
>> - volatile 'val' 'Ljava/lang/Object;' @20
>> - volatile 'next' 'Ljava/util/concurrent/ConcurrentHashMap$Node;' @24 << last field of base class
>> - 'red' 'Z' @28 << derived class starts here, non-oops lead
>> - 'parent' 'Ljava/util/concurrent/ConcurrentHashMap$TreeNode;' @32
>> - 'left' 'Ljava/util/concurrent/ConcurrentHashMap$TreeNode;' @36
>> - 'right' 'Ljava/util/concurrent/ConcurrentHashMap$TreeNode;' @40
>> - 'prev' 'Ljava/util/concurrent/ConcurrentHashMap$TreeNode;' @44
>> - non-static oop maps (2 entries): 16-24 32-44
>>
>> Now:
>>
>> java.util.concurrent.ConcurrentHashMap$TreeNode {0x000000007e1de450}
>> - ---- non-static fields (9 words):
>> - final 'hash' 'I' @12
>> - final 'key' 'Ljava/lang/Object;' @16
>> - volatile 'val' 'Ljava/lang/Object;' @20
>> - volatile 'next' 'Ljava/util/concurrent/ConcurrentHashMap$Node;' @24 << last field of base class
>> - 'parent' 'Ljava/util/concurrent/ConcurrentHashMap$TreeNode;' @28 << class starts here, oop...
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>
> - add regression test
> - Reworked to use prior super klass layout reconstruction pass
> - Merge branch 'master' into JDK-8353273-Reduce-average-number-of-oop-map-entries-in-instance-objects
> - alternate-order
> - print
Changes requested by lmesnik (Reviewer).
test/hotspot/jtreg/runtime/FieldLayout/TestOopMapSizeMinimal.java line 34:
> 32: /*
> 33: * @test id=no_coops_no_ccptr_no_coh
> 34: * @library /test/lib /
Using "/" as testlibrary is not a good pattern. "/" is not a lib. Why is it needed here?
test/hotspot/jtreg/runtime/FieldLayout/TestOopMapSizeMinimal.java line 94:
> 92: static {
> 93: WhiteBox WB = WhiteBox.getWhiteBox();
> 94: boolean is_64_bit = System.getProperty("sun.arch.data.model").equals("64");
I am a little bit confused with this check
and
`*` @requires vm.bits == "64"`
Shouldn't "sun.arch.data.model" be always 64?
-------------
PR Review: https://git.openjdk.org/jdk/pull/24330#pullrequestreview-2741350318
PR Review Comment: https://git.openjdk.org/jdk/pull/24330#discussion_r2027836141
PR Review Comment: https://git.openjdk.org/jdk/pull/24330#discussion_r2027845359
More information about the hotspot-dev
mailing list