[master] RFR: JDK-8325104: Lilliput: Shrink Classpointers [v3]

[Stefan Karlsson]

On Tue, 26 Mar 2024 11:13:29 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/oops/oop.hpp line 345:
>> 
>>> 343:       constexpr int load_shift = markWord::klass_load_shift;
>>> 344:       STATIC_ASSERT(load_shift % 8 == 0);
>>> 345:       return mark_offset_in_bytes() + load_shift / 8;
>> 
>> Isn't this broken for big-endian machines? The follow-up question then is, should we really be reading the klass pointer with 32-bit loads? If we load the entire 64-bit "object header" and then shift with `klass_shift`, we wouldn't have to think about endianess, right? Do we keep the 32-bit load because we don't want to mess with C2?
>
> There was a technical problem when I developed the patch, which is that a klass_offset_in_bytes needed to be != mark_offset_in_bytes. See e.g. `Compile::flatten_alias_type`, but I believe there had been other places.
> 
> Then, a 32-bit load can possibly be encoded with fewer bytes, no? I am not sure if it would be faster beyond that distinction, however.

I still think it is odd that we return that the klass offset is 4 on big-endian machines. If we ever really try to read the klass at (obj + klass_offset_in_bytes()) on those machines we will not get the klass bits, instead we will get some of the other bits in the object header.

My inquiries were more if it really was a good idea to load the klass with 4 byte loads and that it seems safer (more platform independent) to stick with 8 byte loads. When we use 8 byte loads and shifts we don't have to think about endianess.

I glanced in the C2 code how klass_byte_in_offset is used. It looks like most places were `klass_offset_in_bytes()` is used, it is used as a sort of a token to figure out that the load (obj + offset) should be interpreted as a load of the klass. We don't actually use that offset for the load / decode of the klass. Instead we perform an 8 byte load from the start of the object, with associated 8 byte shift operation.

So, maybe my concern here is more that the name "klass_offset_in_bytes" is a bit misleading as it sounds like this is the offset where the klass bits are located. Code using klass_offset_in_bytes() really need to be written with the understanding that this is the case.

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

PR Review Comment: https://git.openjdk.org/lilliput/pull/128#discussion_r1539244106