RFR: 8316694: Implement relocation of nmethod within CodeCache [v38]
Vladimir Kozlov
kvn at openjdk.org
Mon Jul 21 23:16:56 UTC 2025
On Thu, 17 Jul 2025 16:19:51 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 does not modify existing code paths and therefore does not benefit much from existing tests. New tests were created to test the new functionality
>>
>> Additional Testing:
>> - [ ] Linux x64 fastdebug all
>> - [ ] Linux aarch64 fastdebug all
>> - [ ] ...
>
> Chad Rakoczy has updated the pull request incrementally with one additional commit since the last revision:
>
> Require caller to hold locks
I have few comments. And I did not look on tests.
Did you check `src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java` since you added Nmethod reference counter?
src/hotspot/share/code/codeBehaviours.cpp line 46:
> 44: bool DefaultICProtectionBehaviour::is_safe(nmethod* method) {
> 45: return SafepointSynchronize::is_at_safepoint() || CompiledIC_lock->owned_by_self() || method->is_not_installed();
> 46: }
Can you rename `method` to `nm` as we call it in similar code in GCs?
src/hotspot/share/code/nmethod.cpp line 1164:
> 1162: #endif
> 1163: + align_up(debug_info->data_size() , oopSize)
> 1164: + align_up(ImmutableDataReferencesCounterSize , oopSize);
Why you need to realign this? There is no requirement to have spaces before `,`
src/hotspot/share/code/nmethod.cpp line 1630:
> 1628: if (!is_java_method()) {
> 1629: return false;
> 1630: }
This should be first check.
src/hotspot/share/code/nmethod.cpp line 2453:
> 2451: // Free memory if this is the last nmethod referencing immutable data
> 2452: if (get_immutable_data_references_counter() == 1) {
> 2453: os::free(_immutable_data);
You should add assert(get_immutable_data_references_counter() > 0 before `if (counter == 1)`
and zero it when freed.
-------------
PR Review: https://git.openjdk.org/jdk/pull/23573#pullrequestreview-3040057036
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r2220488851
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r2220501624
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r2220527842
PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r2220532609
More information about the hotspot-compiler-dev
mailing list