On Thu, 7 May 2026 17:10:13 GMT, Vladimir Kozlov <[email protected]> wrote:
>> src/hotspot/share/code/aotCodeCache.cpp line 2056:
>>
>>> 2054: log_info(aot, codecache, nmethod)("%d (L%d) '%s' AOT%s is
>>> skipped: method is not in AOT cache",
>>> 2055: compile_id(), comp_level(),
>>> method->name_and_sig_as_C_string(), (for_preload ? " preload" : ""));
>>> 2056: assert(AOTCacheAccess::can_generate_aot_code(method), "sanity");
>>
>> Why do we assert the opposite of the condition that led us into this branch
>> and then return nullptr? If we intend to fail then surely we should be
>> calling ShouldNotReachHere()
>
> In production VM I want just skip this method. In debug VM I want assert. I
> could use `assert(false)` but it is not informative. So I used positive
> condition which guarantee fail because of condition above.
Ok, I would have found `assert(false)` to be clearer but I'm happy to leave it
as is.
> _getEventWriter is there but I am not sure it is used.
`getEventWriter` is a bit of a special case. It is called from generated
bytecode that is added to implement the `commit` method of some JFREvent
subclass. I believe the bytecode transform is only applied when event
generation is enabled for the event subclass and gets removed when event
generation is disabled (the transform is implemented in class
`jfr.internal.EventInstrumentation.java`).
So, assuming JFR is not enabled in the assembly run I think we should never see
a call to that method when compiling AOT code. If it is enabled we might
possibly see a call but that would depend on event generation somehow being
enabled
Also, we might still AOT compile something that declares a JFR event and calls
its `commit` method -- with an empty body -- and end up inlining it. That will
be fine to load and use during production while the event is disabled. If it
was ever enabled the AOT code should get deoptimized due to the bytecode
redefine i.e. just business as usual for JFR.
I am also not clear why the intrinsic provided for `JVM::getClassId` is not
used by libraryKit. The test code includes a comment about waiting until
getClassId "is reworked to accommodate epoch shift tagging". Perhaps @D-D-H
could comment on this?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3217498133
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3217900802
