RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v11]
Johan Sjölen
jsjolen at openjdk.org
Tue Sep 17 10:31:19 UTC 2024
On Tue, 17 Sep 2024 09:59:49 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> src/hotspot/share/memory/classLoaderMetaspace.hpp line 81:
>>
>>> 79: metaspace::MetaspaceArena* class_space_arena() const { return _class_space_arena; }
>>> 80:
>>> 81: bool have_class_space_arena() const { return _class_space_arena != nullptr; }
>>
>> This is unnecessary. Instead of having this and having to remember to check for nullness each time, just change the `_class_space_arena` to point to the same arena as the `_non_class_space_arena` does when we run with `-XX:-UseCompressedClassPointers`
>
> I'd prefer not to.
>
> This logic (when -UCCP class space arena is NULL, with the implicit assumption that both are different entities) has been in there forever, and changing that is out of scope for and unrelated to this PR. I am not sure what will break if I change this but don't want to risk test errors at this point (one example, reporting would have to be adapted to recognize that both arenas are the same, and there are plenty of tests that would also need to be fixd).
>
> This can be done in a follow-up RFE if necessary.
OK, that's fine.
>> src/hotspot/share/memory/metaspace.cpp line 656:
>>
>>> 654: // Adjust size of the compressed class space.
>>> 655:
>>> 656: const size_t res_align = reserve_alignment();
>>
>> Can you change the name to `root_chunk_size`?
>
> It feels wrong, since this is a deeply hidden implementation detail.\
>
> I will remove this temporary variable, which will also make the diff smaller.
Sounds OK, I wanted the name change to indicate that "hey, deep impl detail where we use this to mean something else".
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1762993568
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1762994772
More information about the build-dev
mailing list