RFR: 8359064: Expose reason for marking nmethod non-entrant to JVMCI client [v2]
Doug Simon
dnsimon at openjdk.org
Wed Jun 11 20:03:28 UTC 2025
On Wed, 11 Jun 2025 19:27:38 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:
> I agree that "change" is broad, but is it a bad thing in this context? Shouldn't both sides of JVMCI (if they want to) be able to "monitor" any change that the other side did to an installed code? "invalidationReason" may not be great as well because the field will be also set when the code is installed.
The only change we're talking about is invalidation via `nmethod::make_not_entrant` right? If so, then it really is *only* an invalidation reason.
Why will the field be set when the code is installed? Do you mean it will be initialized to the default `int` value of 0? Then it should be initialized to `-1` to denote that no invalidation has occurred.
All the current reasons are HotSpot specific and the very notion of "change" is exactly tied to `nmethod::make_not_entrant` so I still think this should all be confined to HotSpotNmethod until we have a good use case for lifting it up to InstalledCode. We've learnt through painful experience that making API too broad, too early almost always comes back to bite us. I would also drop the enum on the Java side. It really doesn't add any extra value as shown by `installedCode.getChangeReason() == HotSpotNmethod.ChangeReason.GC_UNLINKING_COLD.ordinal()` [here](https://github.com/JohnTortugo/graal/pull/2/files#diff-1f5e4cd4034f7f7571d9a021272192501800c9b19a41101bc26aaaeebaf14e15R189). That is, if you're using the ordinal of an enum, then the enum is probably just an unnecessary box for an `int`. While you could make the `HotSpotNmethod.invalidationReason` field itself be an enum, that makes setting the field in `JVMCINMethodData::invalidate_nmethod_mirror` significantly more complicated.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25706#issuecomment-2963996747
More information about the graal-dev
mailing list