RFR: 8344363: FullGCForwarding::initialize_flags is called after ObjLayout::initialize [v4]

[Albert Mingkun Yang]

On Thu, 21 Nov 2024 12:44:58 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> From the bug description:
>> ObjLayout::initialize() is called in Arguments::parse(const JavaVMInitArgs*) which sets ObjLayout::_klass_mode. FullGCForwarding::initialize_flags(size_t) is called in init_globals() which seems to be later in the Threads::create_vm(JavaVMInitArgs*, bool*) routine. The latter, however, can unset the UseCompactObjectHeaders flag, which leads to a potential mismatch with ObjLayout::_klass_mode, firing the asserts in ObjLayout::klass_mode().
>> 
>> There is also a related (and somewhat minor) problem: In Arguments::parse() we enable a bunch of stuff when UseCompactObjectHeaders is on (LW locking, but that's the default anyway, and object-monitor-tables, which are otherwise off), but then may disable UseCompactObjectHeaders, e.g. in the GC or when -UseCompressedClassPointers is requested. This then leaves the odd situation that we run without compact headers, but have object monitor tables turned on.
>> 
>> The fix to both issues is:
>> - First disable UCOH
>> - and do that in apply_ergo() (e.g. GCs should do it in GCArguments::initialize())
>> - then enable any flags required by compact headers
>> - Initialize ObjLayout after all flags are done (it's in the correct place, already)
>> 
>> Testing:
>>  - [x] tier1 -UCOH (default)
>>  - [ ] tier1 +UCOH
>>  - [x] Manual testing several flag combinations
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move ObjLayout::initialize() up a little

Marked as reviewed by ayang (Reviewer).

src/hotspot/share/runtime/arguments.cpp line 3660:

> 3658: }
> 3659: 
> 3660: void Arguments::set_compact_headers() {

Since this method does more than setting the compact-headers flag, could it be renamed to `set_compact_headers_flags`, following the convention from its neighbor `set_ergonomics_flags`?

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

PR Review: https://git.openjdk.org/jdk/pull/22255#pullrequestreview-2455137935
PR Review Comment: https://git.openjdk.org/jdk/pull/22255#discussion_r1854326306