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

[Dean Long]

On Mon, 17 Mar 2025 19:48:35 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> I believe this should behave the same as creating any other nmethod. `CodeCache_lock` is the only thing the other constructors use when initializing nmethods and if this was an issue they could also encounter the same race between initialization and setting the field.
>> 
>> Looking through the GC code also shows they do hold `CodeCache_lock` before scans. [G1](https://github.com/openjdk/jdk/blob/19154f7af34bf6f13d61d7a9f05d6277964845d8/src/hotspot/share/gc/g1/g1HeapRegionRemSet.cpp#L112), [Shenandoah](https://github.com/openjdk/jdk/blob/19154f7af34bf6f13d61d7a9f05d6277964845d8/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp#L195), [ZGC](https://github.com/openjdk/jdk/blob/19154f7af34bf6f13d61d7a9f05d6277964845d8/src/hotspot/share/gc/z/zNMethodTable.cpp#L215)
>
> Thank you for checking it.

Other nmethod contructors don't have the same locking requirements, because the nmethod hasn't been registered with GC yet.  However, for the source nmethod, it could be concurrently patched by GC threads without codeCache_lock and only the per-nmethod CompiledICLocker locking mechanism.  So using memcpy() seems problematic here, because a byte-by-byte copy might see on partial updates from NativeCall::set_destination_mt_safe, for example.
Also, there seems to be a critical race with GC here.  The destination nmethod isn't going to be registered with GC yet, correct?  In that case, GC may patch the source nmethod right after the copy, but before the destination copy is registered, leaving the destination copy with stale data.  This seems fatal, as I believe this breaks crucial invariants preventing call sites from referencing stale data.  @fisk, am I on the right track here?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r2000455741