RFR: 8253278: Refactor/cleanup oopDesc::*_klass_addr
Coleen Phillimore
coleenp at openjdk.java.net
Fri Sep 18 12:35:05 UTC 2020
On Fri, 18 Sep 2020 05:49:59 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Right now this one has the comment mentioning CMS:
>>
>> Klass** oopDesc::klass_addr(HeapWord* mem) {
>> // Only used internally and with CMS and will not work with
>> // UseCompressedOops
>> assert(!UseCompressedClassPointers, "only supported with uncompressed klass pointers");
>> ByteSize offset = byte_offset_of(oopDesc, _metadata._klass);
>> return (Klass**) (((char*)mem) + in_bytes(offset));
>> }
>>
>> But that is not true, it is used on the generic paths.
>>
>> The rest are the snowballing changes:
>> - in `load_klass_raw` and `set_klass`: `compressed_klass_addr` and `klass_addr` are used where the direct access to
>> `_metadata` can be done, avoiding these methods
>> - the leftover use is in `set_klass_release`, where we can simplify things by inlining `_addr` methods directly
>> - `CHECK_SET_KLASS` macro seems too much ceremony for just two asserts
>>
>> Note that new code uses the universal `klass_offset_in_bytes`, that is technically `offset_of(..., _metadata._klass)`.
>> This is different from the previous code, but it matches what other code is doing -- it trusts that both
>> `_metadata._klass` and `_metadata._compressed_klass` are on the same offsets. I thought to add a very paranoid assert
>> there, but decided not to. Testing:
>> - [x] Linux x86_64 fastdebug hotspot_gc_shenandoah
>> - [x] Linux x86_64 release hotspot_gc_shenandoah
>> - [x] Linux x86_64 fastdebug tier1
>> - [x] Linux x86_64 fastdebug tier2
>> - [x] Linux x86_64 fastdebug tier1, -XX:+UseShenandoahGC
>> - [x] Linux x86_64 fastdebug tier2, -XX:+UseShenandoahGC
>
> Marked as reviewed by stefank (Reviewer).
> It is a sensitive code, I'd like another reviewer.
Yes, I didn't say it was trivial so yes, you needed another reviewer, which I see you got. Also, there should be an
approximate 24 hr turnaround for PRs so that all the timezones have a chance to review it.
-------------
PR: https://git.openjdk.java.net/jdk/pull/221
More information about the hotspot-dev
mailing list