jdk 8316694: Implement relocation of nmethod within CodeCache

This PR introduces a new function to replace nmethods, addressing JDK-8316694. It enables the creation of new nmethods from existing ones, allowing method relocation in the code heap and supporting 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 only slightly modifies 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
[ ] ...

Progress

[ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
[x] Change must not contain extraneous whitespace
[x] Commit message must refer to an issue

Issue

JDK-8316694: Implement relocation of nmethod within CodeCache (Enhancement - P4)

Reviewers

Tom Rodriguez (@tkrodriguez - Reviewer) 🔄 Re-review required (review applies to a0134a87)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23573/head:pull/23573
$ git checkout pull/23573

Update a local copy of the PR:
$ git checkout pull/23573
$ git pull https://git.openjdk.org/jdk.git pull/23573/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23573

View PR using the GUI difftool:
$ git pr show -t 23573

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23573.diff

Using Webrev

Link to Webrev Comment

Feb 11 '25 22:02 chadrako

:wave: Welcome back chadrako! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Feb 11 '25 22:02 bridgekeeper[bot]

@chadrako This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8316694: Implement relocation of nmethod within CodeCache

Reviewed-by: kvn, eosterlund, never, eastigeevich, bulasevich

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

e6868c624851d5c6bd182e45ba908cb06b731e8c: 8369138: New test compiler/loopstripmining/MissingStoreAfterOuterStripMinedLoop.java fails
837f634bf29fd877dd62a2e0f7d7a1bd383372d3: 8369128: ProblemList jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java in Xcomp configs

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch. As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@vnkozlov, @fisk, @tkrodriguez, @eastig, @bulasevich) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

Feb 11 '25 22:02 openjdk[bot]

@chadrako The following label will be automatically applied to this pull request:

hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

Feb 11 '25 22:02 openjdk[bot]

Webrevs

Mar 11 '25 23:03 mlbridge[bot]

/label add hotspot-compiler

Mar 11 '25 23:03 chadrako

@chadrako The hotspot-compiler label was successfully added.

Mar 11 '25 23:03 openjdk[bot]

/label remove hotspot

Mar 11 '25 23:03 chadrako

@chadrako The hotspot label was successfully removed.

Mar 11 '25 23:03 openjdk[bot]

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

Mar 12 '25 19:03 vnkozlov

Hi @fisk,

Thank you for the very valuable comment. It has point we have not thought about.

I am not fond of "special nmethods" that work subtly different to normal nmethods and have their own special life cycles.

It's not clear to me what you mean "special nmethods". IMO we don't introduce any special nmethods. From my point of view, a normal nmethod is an nmethod for a ordinary Java method. Nmethods for non-ordinary Java methods are special, e.g. native nmethods or method handle linkers(JDK-8263377). I think normal nmethods should be relocatable within CodeCache.

You can't just copy oops.

Yes, this is the main issue at the moment. Can we do this at a safepoint?

I'm worried about copying the nmethod epoch counters

We should clear them. If not, it is a bug.

You don't check if the nmethod is_unloading() when cloning it.

Should such nmethods be not entrant? We don't relocate not entrant nmethods.

Have you checked what the JVMCI speculation data

Good point to check.

By running the operation in a safepoint you a) introduce an obvious latency problem

Yes, we are going to measure it. We don't expect relocation to be a frequent operation.

What are the consequences of copying the deoptimization generation?

What do you mean?

Apr 08 '25 21:04 eastig

Hi @fisk,

Thank you for the very valuable comment. It has point we have not thought about.

I am not fond of "special nmethods" that work subtly different to normal nmethods and have their own special life cycles.

It's not clear to me what you mean "special nmethods". IMO we don't introduce any special nmethods. From my point of view, a normal nmethod is an nmethod for a ordinary Java method. Nmethods for non-ordinary Java methods are special, e.g. native nmethods or method handle linkers(JDK-8263377). I think normal nmethods should be relocatable within CodeCache.

I mean nmethods with a subtly different life cycle where usual invariants/expectations don't hold. Like method handle intrinsics and enter special intrinsics for example. Used to have a different life cycle for OSR nmethods too.

You can't just copy oops.

Yes, this is the main issue at the moment. Can we do this at a safepoint?

I don't think it solves much. You can't stash away a pointer to the nmethod, roll to a safepoint, and expect the nmethod to not be freed. Even if you did, you still can't copy the oops.

If we are to do this, I think you want to apply nmethod entry barriers first. That stabilizes the oops.

I'm worried about copying the nmethod epoch counters

We should clear them. If not, it is a bug.

I'd like to change copying from opt-out to opt-in instead; that would make me feel more comfortable. Then perhaps you can share initialization code that sets up the initial state of the nmethod exactly in the same way as normal nmethods.

I didn't check but you need to take the Compile_lock and verify dependencies too if you didn't do that, I think, so you don't race with deoptimization.

You don't check if the nmethod is_unloading() when cloning it.

Should such nmethods be not entrant? We don't relocate not entrant nmethods.

is_not_entrant doesn't imply is_unloading.

What are the consequences of copying the deoptimization generation?

What do you mean?

I mean is it safe to racingly copy the deoptmization generation when there is concurrent deoptimization? This is why I'd prefer copying to be opt-in rather than opt-out so we don't have to stare at every single field and wonder what will happen when a new nmethod "inherits" state from a different nmethod in interesting races. I want it to work as much as possible as normal nmethod installation, starting with a state as close as possible to when the original nmethod was created, as opposed to its eventually mutated state.

Apr 11 '25 14:04 fisk

I’ve made several adjustments to address the key concerns. The relocation logic now avoids copying stale or invalid metadata by using MethodHandles to maintain valid references. This approach holds the corresponding method for the nmethod being relocated, which should be safe since we only relocate Java methods that have associated method objects. I’ve also added safety checks, including one to prevent relocating unloading nmethods. The relocation is now “opt-in,” creating a new nmethod and copying only the necessary values, which should mitigate the risk of unintentionally carrying over invalid state. I’m still investigating the JVMCI-specific behavior, particularly how the nmethod mirror is handled, and will follow up once I have a clearer understanding.

Apr 24 '25 23:04 chadrako

@fisk Thank you for the valuable feedback. Here is a more detailed response to the concerns you brought up

1 You can't just copy oops. Propagating stale pointers to a new nmethod is not valid and will make the GC vomit. The GC assumes that it can traverse a snapshot of nmethods, and that new nmethods created after that snapshot, will have sane valid oops initially, and hence do not need fixing. Copying stale oops to a new nmethod would violate those invariants and inevitably blow up.

Instead of tracking the nmethod pointer which could become stale I updated the code to use method handles. I believe the method handle should ensure the method remains valid and we can then relocate its corresponding nmethod. Reference

2 Class redefinition tracks in an external data structure which nmethods contained metadata that we want to eventually throw away. This is done to avoid walking the entire code cache just to keep tabs on the one nmethod that still uses the old metadata. If we clone the nmethod without putting it in said data structure, we will blow up.

The relocated nmethod is added as a dependent nmethod on all of the MethodHandles and InstranceKlass in its dependency scope. Reference

3 I'm worried about the initial state of the nmethod entry barrier guard value being copied from the source nmethod, instead of having the initial value we expect for newly created nmethods. It means that the initial invocation will not get the nmethod entry barrier callback. The GC traverses the nmethods assuming that new nmethods created during the traversal will not start off with weird stale values.

The source nmethod entry barrier is now called before copying. I believe this will disarm the barrier and reset the guard value for it to be safe to copy. Reference

4 I'm worried about copying the nmethod epoch counters used by virtual threads to mark which nmethods have been found on-stack. Copying it implies that this nmethod has been found on-stack even though it never has. To me, the implications are unknown, but perhaps you thought about it?

Copying this value was not intentional. It should be correctly set to the default value now. Reference

5 You don't check if the nmethod is_unloading() when cloning it. That means you can create a new nmethod that has dead oops from the get go - that cannot be allowed

I added this check to ensure the nmethod is not unloading and removed the not entrant check as is unloading implies not entrant. Reference

6 Have you checked what the JVMCI speculation data and JVMCI data contains and if your approach will break that? JVMCI has an nmethod mirror object that refers back to the nmethod - this is unlikely to work out of the box with cloning.

I’m still investigating the JVMCI speculation data and how the nmethod mirror is used. I will follow up when I have a clearer understanding.

7 By running the operation in a safepoint you a) introduce an obvious latency problem, b) create a new source for stale nmethod pointers that will become stale and burn. The _nm of the safepoint operation might not survive a safepoint. For example, if a GC safepoint runs first, the GC might decide to unload the nmethod. It then traverses all known pointers to stale nmethods, and cleans them up so that nobody is referring to the nmethod any longer. Naturally, the GC won't know that there is a stale _nm pointer embedded into your VM operation. When you start messing around with it you enter a use-after-free situation and we will blow up.

a) Due to the nature of oops it seems a safe point is necessary. I do not see a fix to the latency problem. b) For the stale nmethod pointer issues I updated to use methodHandle. Same reasoning as number 1

8 What are the consequences of copying the deoptimization generation? I don't know!

This was unintentional and the value is no longer copied. Reference

9 Sometimes the method() is null when using Truffle.

I added null check before updating nmethod reference to help avoid this. As mentioned earlier I do not have much knowledge around Truffle/JVMCI so I will follow up on this when I have a better understanding.

10 Since you don't hold the Compile_lock across the safepoint, it's not obvious to me that you can't get a not_installed nmethod. Can you? I don't know what the consequences are of cloning one of those. The target nmethod will start off as not_installed, but I don't know that it will be made in_use.

I updated the code to hold the Compile_lock to ensure we do not relocate nmethods during construction. Reference

11 These new special nmethods call post_init after installing the nmethod in the Method, while normally the order is reversed. While this may or may not be okay, it introduces a new anomaly where new special nmethods are being special

I moved the post_init call to be more inline with the other constructors. Creating a “special” nmethod was not the intention and I agree the relocation should follow the normal creation where possible. Reference

Apr 25 '25 20:04 chadrako

@fisk Thank you for the valuable feedback. Here is a more detailed response to the concerns you brought up

Thanks, it's shaping up.

Instead of tracking the nmethod pointer which could become stale I updated the code to use method handles. I believe the method handle should ensure the method remains valid and we can then relocate its corresponding nmethod. Reference

The safepoint is still causing more trouble than it solves. It was introduced due to oop phobia. What the oops really needed to stabilize is to run the entry barrier which you do now. The safepoint merely destabilizes the oops again while introducing latency problems and fun class redefinition interactions. It should be removed as I can't see it serves any purpose.

The relocated nmethod is added as a dependent nmethod on all of the MethodHandles and InstranceKlass in its dependency scope. Reference

My concern was about something else - a table tracks all the nmethods that have old metadata in order to speed up a walk over the code cache that finds said nmethods.

This should be dealt with by not relocating nmethods with evol dependencies/metadata and by not safepointing, which could introduce class redefinition which populates this table.

The source nmethod entry barrier is now called before copying. I believe this will disarm the barrier and reset the guard value for it to be safe to copy. Reference

Yes and fix the oops so you don't need a safepoint.

Copying this value was not intentional. It should be correctly set to the default value now. Reference

Good.

I added this check to ensure the nmethod is not unloading and removed the not entrant check as is unloading implies not entrant. Reference

That's not quite true. There are two separate mechanisms that guard the entry. When sn nmethod becomes invalid due to for example a broken speculative assumption, the verified entry is patched with a jump to a trampoline. This is what is_not_entrant refers to and nmethods are made not entrant one by one.

is_unloaded() is a separate mechanism using the nmethod entry barriers which can be bulk armed instead, and uses a conditional branch to guard the entry. This is used by the GC to guard the entry.

You should only relocate nmethods that are is_in_use().

I’m still investigating the JVMCI speculation data and how the nmethod mirror is used. I will follow up when I have a clearer understanding.

Sounds good.

a) Due to the nature of oops it seems a safe point is necessary. I do not see a fix to the latency problem.

b) For the stale nmethod pointer issues I updated to use methodHandle. Same reasoning as number 1

I hope I explained by now that the safepoint doesn't really help with the oops.

This was unintentional and the value is no longer copied. Reference

Good.

I added null check before updating nmethod reference to help avoid this. As mentioned earlier I do not have much knowledge around Truffle/JVMCI so I will follow up on this when I have a better understanding.

Sounds good.

I updated the code to hold the Compile_lock to ensure we do not relocate nmethods during construction. Reference

Okay. Speaking of which, seems like the NMethodState_lock is held for way too long - usually just held when setting the Method code and updating the nmethod state after the initial state is set. Keeping the lock across other things makes me worried of deadlocks.

I moved the post_init call to be more inline with the other constructors. Creating a “special” nmethod was not the intention and I agree the relocation should follow the normal creation where possible. Reference

Thanks.

Apr 25 '25 22:04 fisk

@fisk I believe I have addressed the remaining issues you brought up

The safepoint is still causing more trouble than it solves. It was introduced due to oop phobia. What the oops really needed to stabilize is to run the entry barrier which you do now. The safepoint merely destabilizes the oops again while introducing latency problems and fun class redefinition interactions. It should be removed as I can't see it serves any purpose.

Relocation no longer occurs at a safepoint

My concern was about something else - a table tracks all the nmethods that have old metadata in order to speed up a walk over the code cache that finds said nmethods. This should be dealt with by not relocating nmethods with evol dependencies/metadata and by not safepointing, which could introduce class redefinition which populates this table.

I added the check for evol dependecies/metadata to not relocate them (reference)

Okay. Speaking of which, seems like the NMethodState_lock is held for way too long - usually just held when setting the Method code and updating the nmethod state after the initial state is set. Keeping the lock across other things makes me worried of deadlocks.

The NMethodState_lock is now only held when the state gets updated (reference)

Have you checked what the JVMCI speculation data and JVMCI data contains and if your approach will break that? JVMCI has an nmethod mirror object that refers back to the nmethod - this is unlikely to work out of the box with cloning.

The HotspotNMethod represented by the nmethod mirror is now updated to reflect the new nmethod (reference reference)

May 08 '25 20:05 chadrako

JVMCI compiled nmethods have been excluded from relocation due to concerns about safely updating their mirror fields. Graal can deoptimize methods and update these fields without acquiring locks, which introduces a potential race condition between relocation and field resetting.

One possible solution is to introduce a lock when updating these fields. The lock must not block safepoints, since updates can be triggered from Java code. However, this would require that mirror updates do not hold any _no_safepoint_check locks. This is currently not the case, such as during deoptimization caused by uncommon traps (source).

Another potential approach is to perform relocation at a safepoint, which would guarantee that Graal is not concurrently updating the fields.

For now, excluding JVMCI methods from relocation appears to be the safest option. I’m in the process of writing a JBS issue to track this and eventually re-enable relocation for these methods. If anyone has suggestions for alternative solutions or improvements, I’d greatly appreciate the feedback.

May 22 '25 20:05 chadrako

I'd like to clarify a bit what's actually done here. Some JVMCI compilation can have an associated instance of InstalledCode that has value written into it by hotspot that point at the nmethod* and the verified entry point. If the mirror object is reclaimed by the garbage collector before the nmethod dies, the mirror field will be cleared. Graal may read those fields but will never write them. JVMCI compilations initiated by the CompileBroker will never have an associated mirror. The mirror object is associated with the method at construction time and will never be changed. So it's not necessary to exclude all JVMCI compiled nmethods from this relocation, only ones which have a non-null mirror object.

May 23 '25 17:05 tkrodriguez

I'd like to clarify a bit what's actually done here. Some JVMCI compilation can have an associated instance of InstalledCode that has value written into it by hotspot that point at the nmethod* and the verified entry point. If the mirror object is reclaimed by the garbage collector before the nmethod dies, the mirror field will be cleared. Graal may read those fields but will never write them. JVMCI compilations initiated by the CompileBroker will never have an associated mirror. The mirror object is associated with the method at construction time and will never be changed. So it's not necessary to exclude all JVMCI compiled nmethods from this relocation, only ones which have a non-null mirror object.

Thanks for this clarification. I have updated it to only exclude nmethod with mirrors.

I want to confirm that is okay to pass false for phatom_ref in get_nmethod_mirror. We don't read or write to the mirror and only call that to see if one exists so I believe it should be okay. Another option is to add a function to return the mirror_index and that can be used as the check instead.

May 27 '25 18:05 chadrako

We'll address this flag in https://bugs.openjdk.org/browse/JDK-8357619 as I think it no longer serves a purpose other than confusion. Passing false should be correct, but I think we'll probably end up adding this:

diff --git a/src/hotspot/share/jvmci/jvmciRuntime.hpp b/src/hotspot/share/jvmci/jvmciRuntime.hpp
index 884d11f792e..0efc957aa88 100644
--- a/src/hotspot/share/jvmci/jvmciRuntime.hpp
+++ b/src/hotspot/share/jvmci/jvmciRuntime.hpp
@@ -117,6 +117,11 @@ class JVMCINMethodData : public ResourceObj {
   // Gets the JVMCI name of the nmethod (which may be null).
   const char* name() { return _has_name ? (char*)(((address) this) + sizeof(JVMCINMethodData)) : nullptr; }
 
+  // Returns true if this nmethod has a mirror
+  bool has_mirror() const {
+    return _nmethod_mirror_index != -1;
+  }
+
   // Clears the HotSpotNmethod.address field in the  mirror. If nm
   // is dead, the HotSpotNmethod.entryPoint field is also cleared.
   void invalidate_nmethod_mirror(nmethod* nm);

which you could adopt and use for your change.

May 27 '25 19:05 tkrodriguez

All previously raised concerns have been addressed. When you have a moment, I’d appreciate a review. Thanks!

May 27 '25 21:05 chadrako

I filed JDK-8357926 to track progress for allowing JVMCI nmethods with mirrors to be relocated

May 27 '25 23:05 chadrako

So this copying keeps the same compile_id, which sort of makes sense but it's also potentially confusing. What's the plan for how this interacts with flags like PrintNMethods and JVMTI code installation notification? This is done in nmethod::post_compiled_method which doesn't seem to be used on the new nmethod. If the reclamation of the old nmethod is performed in the normal fashion, we now have 2 nmethods alive with the same compile_id which could be confusing. But allocating a new compile_id breaks the connection to the original compile which seems bad too.

May 30 '25 19:05 tkrodriguez

So this copying keeps the same compile_id, which sort of makes sense but it's also potentially confusing. What's the plan for how this interacts with flags like PrintNMethods and JVMTI code installation notification? This is done in nmethod::post_compiled_method which doesn't seem to be used on the new nmethod. If the reclamation of the old nmethod is performed in the normal fashion, we now have 2 nmethods alive with the same compile_id which could be confusing. But allocating a new compile_id breaks the connection to the original compile which seems bad too.

As we are not compiling, compile_id should stay the same. Yes, we need to add some logging: log_info(codecache) and PrintNMethods.

According to https://docs.oracle.com/en/java/javase/24/docs/specs/jvmti.html#CompiledMethodLoad, compile methods can be moved. We need to generate events if it happens:

If it is moved, the CompiledMethodUnload event is sent, followed by a new CompiledMethodLoad event.

we now have 2 nmethods alive with the same compile_id which could be confusing.

If compile_id is interpreted as id of nmethod, it is confusing. Comment to nmethod::_compile_id: https://github.com/openjdk/jdk/blob/aea2837143289800cfbb7044de4f105e87e233ff/src/hotspot/share/code/nmethod.hpp#L259

According to it, it is id of a compilation task. In such case there should be no confusion.

Jun 02 '25 17:06 eastig

Thanks for pointing out the missing JVMTI event publication. I’m currently looking into what’s required to address that, along with JFR event publication that may also have been missed. I’d appreciate hearing others’ thoughts on how critical this is: should we treat it as a blocker for integration, or would it be acceptable to follow up with a separate issue?

We’re hoping to get this into JDK 25, as it would simplify both development and backporting of features related to hot code grouping. That said, if the consensus is that JVMTI/JFR support is essential upfront, this can be delayed until JDK 26.

Jun 02 '25 22:06 chadrako

We’re hoping to get this into JDK 25, as it would simplify both development and backporting of features related to hot code grouping. That said, if the consensus is that JVMTI/JFR support is essential upfront, this can be delayed until JDK 26.

I don't think this can be put into JDK 25. Too late and changes are not simple. And yes, JVMTI/JFR support is essential - you have to support all public functionalities of VM.

Jun 03 '25 00:06 vnkozlov

If it is moved, the CompiledMethodUnload event is sent, followed by a new CompiledMethodLoad event.

we now have 2 nmethods alive with the same compile_id which could be confusing.

It's nice that the JVMTI docs considered this problem but the notifications will be sent in the reverse order given our current implementation. We will create a new nmethod while the old nmethod might still be alive, at least for the purposes of deopt. Since this PR doesn't actually perform any relocation, I'm not sure what the plan is here. The most aggressive thing that could be done is to invalidate all frames which have the old nmethod on stack, but that still leaves the nmethod live for the purposes of deopt. It would probably be ok to synthesize an unload after the deopt since there should be no actual execution in those nmethods, but you will then have to suppress the one that's normally done during nmethod::unlink.

I agree that the docs are fairly clear that all of this is ok, but that doesn't mean that assumptions haven't been made about the current implementation. We just need to make sure we do something rational and that it's possible to understand from our output what was done.

Jun 03 '25 15:06 tkrodriguez

Since this PR doesn't actually perform any relocation, I'm not sure what the plan is here.

The plan is to use this functionality in JDK-8326205

The most aggressive thing that could be done is to invalidate all frames which have the old nmethod on stack, but that still leaves the nmethod live for the purposes of deopt. It would probably be ok to synthesize an unload after the deopt since there should be no actual execution in those nmethods, but you will then have to suppress the one that's normally done during nmethod::unlink.

This might have negative performance impact because we will be relocating hot nmethods. IMO it's better to let calls of the original nmethod to finish. New calls will be using the copy.

It looks like the implementation does not move code in the terms of the JVMTI spec. The JVMTI spec expects moving code to unload it from memory:

Compiled Method Unload

Sent when a compiled method is unloaded from memory.

As we don't want to unload code from memory, we cannot send Compiled Method Unload event.

I think we can generate just Compiled Method Load event because of the note:

Note that a single method may have multiple compiled forms, and that this event will be sent for each form.

Alternatively, we can update the JVMTI spec to say Compiled Method Load event can be a result of code copied.

Jun 17 '25 15:06 eastig

@chadrako this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8316694-Final
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

Jun 17 '25 20:06 openjdk[bot]

⚠️ @chadrako This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

Jun 17 '25 20:06 openjdk[bot]

@tkrodriguez

It's nice that the JVMTI docs considered this problem but the notifications will be sent in the reverse order given our current implementation. We will create a new nmethod while the old nmethod might still be alive, at least for the purposes of deopt. Since this PR doesn't actually perform any relocation, I'm not sure what the plan is here.

I actually think it is better if we do not send unload events during relocation and allow the current code path (nmethod::unlink()) to send the unload event. Like you mentioned "relocation" isn’t actually moving the nmethod as it just copies and invalidates the original. According to the JVMTI spec:

Note that a single method may have multiple compiled forms, and that this [CompiledMethodLoad] event will be sent for each form

So it should be safe for us to do this.

The most aggressive thing that could be done is to invalidate all frames which have the old nmethod on stack, but that still leaves the nmethod live for the purposes of deopt. It would probably be ok to synthesize an unload after the deopt since there should be no actual execution in those nmethods, but you will then have to suppress the one that's normally done during nmethod::unlink.

With the updated approach I don’t think this is necessary. Since the original nmethod is only being marked as not entrant it is still safe for it to be “live”. This allows us to avoid special cases for relocated nmethods and it should behave similarly to as if it were recompiled.

I also added a test to verify that we correctly publish the load and unload events during relocation (source)

Jun 19 '25 22:06 chadrako

jdk jdk copied to clipboard

8316694: Implement relocation of nmethod within CodeCache

Progress

Issue

Reviewers

Reviewing

Webrevs

jdk
jdk copied to clipboard