RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v11]

[Coleen Phillimore]

On Wed, 18 Sep 2024 13:57:29 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/memory/classLoaderMetaspace.cpp line 87:
>> 
>>> 85:         klass_alignment_words,
>>> 86:         "class arena");
>>> 87:   }
>> 
>> As per my comment in the header file, change the code to this:
>> 
>> ```c++
>> if (class_context != nullptr) {
>>   // ... Same as in PR
>> } else {
>>   _class_space_arena = _non_class_space_arena;
>> }
>
> Rather not, see reasoning under https://github.com/openjdk/jdk/pull/20677/files#r1754330432

Yes, I'd rather _class_space_arena be nullptr if not used.

>> src/hotspot/share/memory/classLoaderMetaspace.cpp line 115:
>> 
>>> 113:   if (wastage.is_nonempty()) {
>>> 114:     non_class_space_arena()->deallocate(wastage);
>>> 115:   }
>> 
>> This code reads a bit strangely. I understand *what* it tries to do. It tries to give back any wasted memory from either the class space arena *or* the non class space arena to the non class space arena's freelist. I assume that we do this since any wastage is presumably too small to be used by our new 22-bit class pointers. However, this context will be lost on future readers. It should have at least a comment in the `if (wastage.is_nonempty())` clause explaining what we expect should happen and why. For example:
>> 
>> ```c++
>> // Any wasted memory is presumably too small for any class.
>> // Therefore, give it back to the non-class space arena's free list.
>
> Yes. Some background:
> 
> - wastage can only occur for larger Klass* alignments (aka class space arena alignment property), so only for +COH (note to self, maybe assert)
> - wastage is, by definition, not aligned to the required Klass* alignment, so it cannot be reused. Yes, its probably also too small
> 
> Yes, I will write a better comment.

Yes, this definitely needs a comment why since this is how we allocate small chunks of wasted because of hyper-aligning Klasses in class space.   Line 111 is somewhat surprising though.  I didn't expect there to be wastage from allocating to non-class-metaspace.

The unnerving bit of this is that CompressedKlassPointers::is_encodable() is true for memory allocated here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1768897591
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1768966812