RFR: 8343789: Move mutable nmethod data out of CodeCache [v14]

[Doug Simon]

On Thu, 6 Mar 2025 12:15:52 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> This change relocates mutable data (such as relocations, metadata, jvmci data) from the nmethod. The change follows the recent PR #18984, which relocated immutable nmethod data out of the CodeCache.
>> 
>> OOPs was initially moved to a new mutable data blob, but then moved back to nmethod due to performance issues on dacapo benchmarks on aarch with ShenandoagGC (why Shenandoah: it is the only GC with supports_instruction_patching=false - it requires loading from the oops table in compiled code, which takes three instructions for a remote data).
>> 
>> Although performance is not the main focus, testing on AArch64 CPUs, where code density plays a significant role, has shown a 1–2% performance improvement in specific scenarios, such as the CodeCacheStress test and the Renaissance Dotty benchmark.
>> 
>> The numbers. Immutable data constitutes **~30%** on the nmehtod. Mutable data constitutes **~8%** of nmethod. Example (statistics collected on the CodeCacheStress benchmark):
>> - nmethod_count:134000, total_compilation_time: 510460ms
>> - total allocation time malloc_mutable/malloc_immutable/CodeCache_alloc: 62ms/114ms/6333ms,
>> - total allocation size (mutable/immutable/nmentod): 64MB/192MB/488MB
>> 
>> Functional testing: jtreg on arm/aarch/x86.
>> Performance testing: renaissance/dacapo/SPECjvm2008 benchmarks.
>> 
>> Alternative solution (see comments): In the future, relocations can be moved to _immutable_data.
>
> Boris Ulasevich has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
> 
>  - swap matadata and jvmci data in outputs according to data layout
>  - cleanup
>  - returning oops back to nmethods. jtreg: Ok, performance: Ok. todo: cleanup
>  - Address review comments: cleanup, move fields to avoid padding, fix CodeBlob purge to call os::free, fix nmethod::print, update Layout description
>  - add a separate adrp_movk function to to support targets located more than 4GB away
>  - Force the use of movk in combination with adrp and ldr instructions to address scenarios
>    where os::malloc allocates buffers beyond the typical ±4GB range accessible with adrp
>  - Fixing TestFindInstMemRecursion test fail with XX:+StressReflectiveCode option:
>    _relocation_size can exceed 64Kb, in this case _metadata_offset do not fit into int16.
>    Fix: use _oops_size int16 field to calculate metadata offset
>  - removing dead code
>  - a bit of cleanup and addressing review suggestions
>  - rework movoop for not_supports_instruction_patching case: correcting in ldr_constant and relocations fixup
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/cfab88b1...bc8c590c

src/hotspot/share/code/nmethod.cpp line 1505:

> 1503:     CHECKED_CAST(_oops_size, uint16_t, align_up(code_buffer->total_oop_size(), oopSize));
> 1504:     uint16_t metadata_size = (uint16_t)align_up(code_buffer->total_metadata_size(), wordSize);
> 1505:     JVMCI_ONLY(CHECKED_CAST(_jvmci_data_size, uint16_t, align_up(compiler->is_jvmci() ? jvmci_data->size() : 0, oopSize)));

This cast is lossy in that `jvmci_data->size()` returns an int. It caused a `double free or corruption (out)` crash in Graal in the case where a `JVMCINMethodData` had a very long name. We've fixed this by [limiting the length of the name](https://github.com/openjdk/jdk/pull/24753) but I'm wondering if there was some special reason for this cast?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r2060115430