RFR: 8343789: Move mutable nmethod data out of CodeCache

[Vladimir Kozlov]

On Tue, 1 Oct 2024 02:10:37 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

> This change relocates mutable data (such as relocations, oops, and metadata) from the nmethod. The change follows the recent PR #18984, which relocated immutable nmethod data from the CodeCache.
> 
> The core idea remains the same: use the CodeCache for executable code while moving additional data to the C heap. The primary motivations are improving security and enhancing code density.
> 
> 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.

First, thank you for doing his work.

Main question is: why you did it only for `nmethod`?

Second question: do you see any performance effects with this change?
My concern is that we iterate relocation info data from different memory space to patch code.

Note, with https://bugs.openjdk.org/browse/JDK-8334691 and other changes I moving into direction to make relocation info data immutable. It is already "immutable" in mainline JDK after https://bugs.openjdk.org/browse/JDK-8333819. But it is still mutable in Leyden because we have to patch indexes during publishing nmethod.

My idea was to move relocation info data (which has big size) into `immutable` data section of nmethod. And leave mutable `_oops` and `_metadata` together with code since they are relatively small and we need to patch them together with code.

Mutable sizes % do not add up:

mutable data    = 6071648 (9.396180%)
   relocation    = 3437176 (12.846409%)
   oops          = 239488 (0.895084%)
   metadata      = 2394984 (8.951227%)

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5273:

> 5271:   } else {
> 5272:     address dummy = address(uintptr_t(pc()) & -wordSize); // A nearby aligned address
> 5273:     mov(dst, Address(dummy, rspec));

Why this is needed?

src/hotspot/share/code/codeBlob.cpp line 85:

> 83: CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, CodeBuffer* cb, int size, uint16_t header_size,
> 84:                    int16_t frame_complete_offset, int frame_size, OopMapSet* oop_maps, bool caller_must_gc_arguments,
> 85:                    bool external_mutable_data) :

I suggest to add `assert(!external_mutable_data || (kind == CodeBlobKind::Nmethod)`

src/hotspot/share/code/codeBlob.hpp line 129:

> 127:   address _mutable_data;
> 128:   int     _mutable_data_size;
> 129: 

Should we add special CodeBlob subclass for nmethod to avoid increase of size for all blobs and stubs?

src/hotspot/share/runtime/vmStructs.cpp line 596:

> 594:   nonstatic_field(nmethod,                     _immutable_data_size,                          int)                                   \
> 595:   nonstatic_field(nmethod,                     _mutable_data,                                 address)                               \
> 596:   nonstatic_field(nmethod,                     _mutable_data_size,                            int)                                   \

They are filed in CodeBlob and not in nmethod.

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

PR Review: https://git.openjdk.org/jdk/pull/21276#pullrequestreview-2422325042
PR Comment: https://git.openjdk.org/jdk/pull/21276#issuecomment-2463441925
PR Comment: https://git.openjdk.org/jdk/pull/21276#issuecomment-2463443197
PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1833476281
PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1833478853
PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1833482205
PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1833483559