RFR: 8316694: Implement relocation of nmethod within CodeCache

[Evgeny Astigeevich]

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.

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");

We need to move this assert to `trampoline_stub_Relocation::fix_relocation_after_move`.
`CodeBuffer::blob()` returns `nullptr` if it wraps `nmethod`.
The modified assert will be:

void trampoline_stub_Relocation::fix_relocation_after_move(const CodeBuffer* src, CodeBuffer* dest) {
  // Finalize owner destination only for nmethods
  if (dest->blob() != nullptr) return;
  // We either relocate a nmethod residing in CodeCache or just generated code from CodeBuffer
  assert(src->blob() != nullptr || nativeCall_at(owner())->raw_destination() == owner(), "destination should be empty");
  pd_fix_owner_after_move();
}

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 1382:

> 1380: 
> 1381:     // First instruction must be a nop as it may need to be patched after relocation
> 1382:     __ nop();

Could you please explain what a problem it fixes?

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

> 1515:     _method->clear_entry_points();
> 1516:     _method->set_code(mh, this);
> 1517:   }

Why do we need this code? Why not to do the same when when we replace C1 nmethod with C2 nmethod?

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

> 1564:   nm->make_deoptimized();
> 1565:   nm->flush_dependencies();
> 1566:   nm->set_is_unlinked();

Why are you explicitly making these calls?

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

> 494:   );
> 495: 
> 496:   static nmethod* replace_nmethod(nmethod* nm, int comp_level_override=-1);

I think we need a function with the name reflecting our purpose:

 // Relocate the nmethod to the code heap identified by code_blob_type.
 // Returns nullptr if the code heap does not have enough space, otherwise
 // the relocated nmethod. The original nmethod will be invalidated.
 // If nm is already in the needed code heap, it is not relocated and the function returns it.
 static nmethod* relocate_to(nmethod* nm, CodeBlobType code_blob_type);

test/hotspot/jtreg/compiler/whitebox/ReplaceNMethod.java line 68:

> 66:         NMethod origNmethod = NMethod.get(method, false);
> 67: 
> 68:         WHITE_BOX.replaceNMethod(method, false);

I suggest to introduce `relocateNMethodTo`:

WHITE_BOX.relocateNMethodTo(method, BlobType.MethodNonProfiled);

test/lib/jdk/test/whitebox/WhiteBox.java line 497:

> 495:     Objects.requireNonNull(method);
> 496:     replaceNMethod0(method, isOsr, -1);
> 497:   }

We don't support relocation of OSR nmethods.

test/lib/jdk/test/whitebox/WhiteBox.java line 499:

> 497:   }
> 498:   public native void     replaceAllNMethods();
> 499:   public native long     getNumNMethods();

We don't need this methods.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1964403179
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1981967623
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1982131418
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1974379846
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1960711698
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1982262476
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1982175396
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r1982174461