RFR: 8316694: Implement relocation of nmethod within CodeCache

[Vladimir Kozlov]

On Tue, 11 Feb 2025 22:05:13 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.

Few comments.

Note, you can use `memcpy` because we don't have nmethod's virtual pointer anymore.

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

> 1394: }
> 1395: 
> 1396: nmethod::nmethod(nmethod& nm) : CodeBlob(nm.name(), CodeBlobKind::Nmethod, nm.size(), nm.header_size())

Should this be `clone()` method instead of constructor. Then you will not need `new()`.

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

> 1397: {
> 1398:   debug_only(NoSafepointVerifier nsv;)
> 1399:   assert_locked_or_safepoint(CodeCache_lock);

Is this lock enough to prevent GC scan it before you finish initializing it?

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

> 1404:     _oop_maps                   = nm.oop_maps()->clone();
> 1405:   }
> 1406:   _relocation_size              = nm._relocation_size;

Did you consider to use `memcpy()` and update only changed fields?

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

> 1512: 
> 1513:   // Copy all nmethod data outside of header
> 1514:   memcpy(content_begin(), nm.content_begin(), nm.size() - nm.header_size());

You would not need it if you `memcpy` whole nmethod.

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

> 1593: }
> 1594: 
> 1595: bool nmethod::is_relocatable() const {

Native nmethods should be skipped too. May be also check `is_in_use()`.

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

PR Review: https://git.openjdk.org/jdk/pull/23573#pullrequestreview-2679552647
PR Comment: https://git.openjdk.org/jdk/pull/23573#issuecomment-2718841950
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1992116881
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1992127297
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1992104534
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1992111015
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1992115388