RFR: 8253278: Refactor/cleanup oopDesc::*_klass_addr

[Aleksey Shipilev]

On Thu, 17 Sep 2020 11:47:19 GMT, Coleen Phillimore <coleenp 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: Linux x86_64 fastdebug tier1
>
> Nice cleanup!!  I'm happy to see klass_addr go. It took me a while to reconcile that klass_offset_in_bytes points to
> _metadata._compressed_klass but I believe it's correct, and used everywhere else so it better be.

It is a sensitive code, I'd like another reviewer.

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

PR: https://git.openjdk.java.net/jdk/pull/221