RFR: 8316694: Implement relocation of nmethod within CodeCache [v4]

[Vladimir Kozlov]

On Fri, 14 Mar 2025 21:57:21 GMT, Chad Rakoczy <duke at openjdk.org> wrote:

>> This PR introduces a new function to replace nmethods, addressing [JDK-8316694](https://bugs.openjdk.org/browse/JDK-8316694). It enables the creation of new nmethods from existing ones, allowing method relocation in the code heap and supporting [JDK-8328186](https://bugs.openjdk.org/browse/JDK-8328186).
>> 
>> When an nmethod is replaced, a deep copy is performed. The corresponding Java method is updated to reference the new nmethod, while the old one is marked as unused. The garbage collector handles final cleanup and deallocation.
>> 
>> This change does not modify existing code paths and therefore does not benefit much from existing tests. New tests were created and confirmed to pass on x64/aarch64 for slowdebug/fastdebug/release.
>
> Chad Rakoczy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix build issues

Few new comments.

make/hotspot/lib/CompileJvm.gmk line 201:

> 199:     DISABLED_WARNINGS_gcc_jvmtiTagMap.cpp := stringop-overflow, \
> 200:     DISABLED_WARNINGS_gcc_macroAssembler_ppc_sha.cpp := unused-const-variable, \
> 201:     DISABLED_WARNINGS_gcc_nmethod.cpp := class-memaccess, \

Why you need this?

src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp line 93:

> 91: void trampoline_stub_Relocation::pd_fix_owner_after_move() {
> 92:   NativeCall* call = nativeCall_at(owner());
> 93:   assert(call->raw_destination() == owner(), "destination should be empty");

Why it was removed?

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

> 1405:   // Increment number of references to immutable data to share it between nmethods
> 1406:   if (immutable_data_size() > 0) {
> 1407:     (*immutable_data_references_begin())++;

Please use nmethod's inline function to update counter which use cast to `int`.

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

> 1707:     }
> 1708: #endif
> 1709:     memset(immutable_data_references_begin(), 1, oopSize);

Don't use `memset`, use nmethod's inline function to set counter which use correct cast to `int`.

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

> 2287: 
> 2288:   if (_immutable_data != blob_end()) {
> 2289:     long _immutable_data_references = *immutable_data_references_begin();

Why `long`? It should be int for all platforms (we still have 32-bit arm).
Check the current value and update it should be done by nmethod's inline functions.

src/hotspot/share/code/nmethod.hpp line 253:

> 251:   int      _speculations_offset;
> 252: #endif
> 253:   int      _immutable_data_references_offset;

Do we really need this? Can we simply use `_immutable_data_end - sizeof(int)` and make sure that `_immutable_data_end` has big enough alignment (which you have already)

src/hotspot/share/code/nmethod.hpp line 338:

> 336:           );
> 337: 
> 338:   nmethod(nmethod& nm);

No need this

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

PR Review: https://git.openjdk.org/jdk/pull/23573#pullrequestreview-2687109649
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996454101
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996454602
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996469711
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996470327
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996471618
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996469194
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1996461584