RFR: 8292818: replace 96-bit representation for field metadata with variable-sized streams [v4]

Frederic Parain fparain at openjdk.org
Wed Mar 15 13:45:46 UTC 2023


On Tue, 14 Mar 2023 01:25:01 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Frederic Parain has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixes includes and style
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Field.java line 75:
> 
>> 73:     int initialValueIndex;
>> 74:     int genericSignatureIndex;
>> 75:     int contendedGroup;
> 
> It seems that these should all be shorts. All the getter methods are casting them to short.

Indexes in the constant pool are unsigned shorts, but Java shorts are signed, using ints is the simplest way to store those indexes.

> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java line 108:
> 
>> 106:     CLASS_STATE_INITIALIZATION_ERROR = db.lookupIntConstant("InstanceKlass::initialization_error").intValue();
>> 107:     // We need a new fieldsCache each time we attach.
>> 108:     fieldsCache = new HashMap<Address, Field[]>();
> 
> This should probably be a WeakHashMap. I tried it and it seems to work (or at least didn't cause any problems). However, when doing a heap dump I didn't notice the table being any smaller on exit when it was made weak, even though there were numerous GC's while dumping the heap.
> 
> The `<key>` is the Address of the hotspot InstanceKlass instance, and this Address is referenced by the SA InstanceKlass mirror. So theoretically when the reference to the mirror goes way, then the cache entry can be cleared.

I've changed the map to a WeakHashMap and didn't see any issue during testing. But I didn't measure footprint.

> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java line 325:
> 
>> 323: 
>> 324:   public int getFieldOffset(int index) {
>> 325:     return (int)getField(index).getOffset();
> 
> Cast to int is not needed

Other APIs (like MetadaField) are using longs to pass offsets, doing a cast here is less disruptive than changing all the other APIs.

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

PR: https://git.openjdk.org/jdk/pull/12855


More information about the serviceability-dev mailing list