RFR: 8353273: Reduce number of oop map entries in instances [v2]

[Johan Sjölen]

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

Hi,

Neat change. I have read up on how field layouts work, and this seems correct to me. Let me run this in our CI for a bit and see what comes out before I approve it, thank you.

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

PR Review: https://git.openjdk.org/jdk/pull/24330#pullrequestreview-2739627906