8348203: [JVMCI] Make eager JVMCI initialization observable in the debugger #23219

simonis · 2025-01-21T16:59:05Z

I realized that I can't debug (i.e. with a Java debugger) the Graal compiler initialization if it happens eagerly (e.g. with EagerJVMCI, JVMCIPrintProperties, JVMCILibDumpJNIConfig). Not that this is something I need every day, but I think it can easily be fixed by moving the early initializing a little further down, right after JvmtiExport::post_vm_initialized():

 diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp
index 1cab9bc5d53..191409c22e3 100644
--- a/src/hotspot/share/runtime/threads.cpp
+++ b/src/hotspot/share/runtime/threads.cpp
@@ -806,12 +806,6 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
   }
 #endif
 
-#if INCLUDE_JVMCI
- if (force_JVMCI_initialization) {
- JVMCI::initialize_compiler(CHECK_JNI_ERR);
- }
-#endif
-
   if (NativeHeapTrimmer::enabled()) {
     NativeHeapTrimmer::initialize();
   }
@@ -826,6 +820,12 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
   // Notify JVMTI agents that VM initialization is complete - nop if no agents.
   JvmtiExport::post_vm_initialized();
 
+#if INCLUDE_JVMCI
+ if (force_JVMCI_initialization) {
+ JVMCI::initialize_compiler(CHECK_JNI_ERR);
+ }
+#endif
+
   JFR_ONLY(Jfr::on_create_vm_3();)
 
 #if INCLUDE_MANAGEMENT

Progress

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

Issue

JDK-8348203: [JVMCI] Make eager JVMCI initialization observable in the debugger (Bug - P3)

Reviewers

Doug Simon (@dougxc - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23219

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…bugger

bridgekeeper · 2025-01-21T16:59:52Z

👋 Welcome back simonis! 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.

openjdk · 2025-01-21T17:00:53Z

@simonis 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:

8348203: [JVMCI] Make eager JVMCI initialization observable in the debugger

Reviewed-by: dnsimon

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 111 new commits pushed to the master branch:

9c55e25: 8347981: RISC-V: Add Zfa zli imm loads
e20bd01: 8344361: Restore null return for invalid services from legacy providers
5a0bdd0: 8346890: AArch64: Type profile counters generate suboptimal code
4a375e5: 8344232: [PPC64] secondary_super_cache does not scale well: C1 and interpreter
0df9dcb: 8346572: Check is_reserved() before using ReservedSpace instances
a09f06d: 8348265: RMIConnectionImpl: Remove Subject.callAs on MarshalledObject
0395593: 8346751: Internal java compiler error with type annotations in constants expression in constant fields
2daafe4: 8348283: java.lang.classfile.components.snippets.PackageSnippets shipped in java.base.jmod
50ca450: 8340784: Remove PassFailJFrame constructor with screenshots
416d469: 8347008: beancontext package spec does not clearly explain why the API is deprecated
... and 101 more: https://git.openjdk.org/jdk/compare/8bfa54055013ca92696982c86ed3869627718219...master

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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-01-21T17:01:26Z

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

hotspot-runtime

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.

mlbridge · 2025-01-21T17:03:49Z

Webrevs

00: Full (111a2a38)

simonis · 2025-01-21T18:05:57Z

/label compiler

openjdk · 2025-01-21T18:06:44Z

@simonis
The compiler label was successfully added.

dholmes-ora

It is not at all obvious what other impacts moving this could have. Is it really appropriate to enter the live phase and claim the VM is initialized if we may error out if JVMCI initialization fails after that point.

What prevents you from doing the debugging you want to do? And why are you "debugging" Graal compiler initialization anyway?

simonis · 2025-01-22T14:46:36Z

It is not at all obvious what other impacts moving this could have. Is it really appropriate to enter the live phase and claim the VM is initialized if we may error out if JVMCI initialization fails after that point.

Yes, because that's the normal case. Usually, JVMCI gets initialized lazily, before its first usage which is much after VMInit was posted. We are looking at a special case here because this patch only moves eager JVMCI initialization (which happens with -XX:+EagerJVMCI, -XX:+JVMCIPrintProperties and -XX:+JVMCILibDumpJNIConfig) after the VMInit event was posted.

What prevents you from doing the debugging you want to do?

Very simple - the debugger is not starting before it gets the VMInit event.

And why are you "debugging" Graal compiler initialization anyway?

Is this a serious question? Because I'm hunting a bug in eager JVMCI initialization.

dholmes-ora · 2025-01-23T06:02:45Z

Yes, because that's the normal case. Usually, JVMCI gets initialized lazily,

Okay that alleviates my concern. I will leave it for JVMCI folk to approve.

Is this a serious question? Because I'm hunting a bug in eager JVMCI initialization.

Yes a serious question but perhaps poorly framed. I was trying to gauge whether it makes sense to change the way the VM is doing this to address the debugging problem that you ran into i.e was this a general issue to be solved, or a one-of special case that should be handled by a local change instead.

dougxc · 2025-01-24T12:13:27Z

Since this is the non-default mode of execution (JVMCI initialization is normally lazy as Volker pointed out), I think this change is fine.

simonis · 2025-01-24T15:36:54Z

Is this a serious question? Because I'm hunting a bug in eager JVMCI initialization.

Yes a serious question but perhaps poorly framed. I was trying to gauge whether it makes sense to change the way the VM is doing this to address the debugging problem that you ran into i.e was this a general issue to be solved, or a one-of special case that should be handled by a local change instead.

Got it. I obviously handled this by a local change first. But I think the JVMCI compiler initialization executes quite some Java code and I think it is beneficial if this code executions is observable by Java agents (which don't include only the debugger).

simonis · 2025-01-24T15:37:19Z

/integrate

openjdk · 2025-01-24T15:37:53Z

Going to push as commit 76f792b.
Since your change was applied there have been 115 commits pushed to the master branch:

909cef5: 8339891: Several sun/security/ssl/SSLSessionImpl/* tests override test.java.opts
56a1877: 8348170: Unnecessary Hashtable usage in CSS.styleConstantToCssMap
9768f60: 8345249: Apply some conservative cleanups in FileURLConnection
c5a69b6: 8331723: Serial: Remove the unused parameter of the method SerialHeap::gc_prologue
9c55e25: 8347981: RISC-V: Add Zfa zli imm loads
e20bd01: 8344361: Restore null return for invalid services from legacy providers
5a0bdd0: 8346890: AArch64: Type profile counters generate suboptimal code
4a375e5: 8344232: [PPC64] secondary_super_cache does not scale well: C1 and interpreter
0df9dcb: 8346572: Check is_reserved() before using ReservedSpace instances
a09f06d: 8348265: RMIConnectionImpl: Remove Subject.callAs on MarshalledObject
... and 105 more: https://git.openjdk.org/jdk/compare/8bfa54055013ca92696982c86ed3869627718219...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-24T15:38:07Z

@simonis Pushed as commit 76f792b.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

dougxc · 2025-05-07T20:05:02Z

FYI: https://bugs.openjdk.org/browse/JDK-8356447

8348203: [JVMCI] Make eager JVMCI initialization observable in the de…

111a2a3

…bugger

openjdk bot added the rfr Pull request is ready for review label Jan 21, 2025

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Jan 21, 2025

openjdk bot added the compiler compiler-dev@openjdk.org label Jan 21, 2025

dholmes-ora reviewed Jan 22, 2025

View reviewed changes

dougxc approved these changes Jan 24, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 24, 2025

openjdk bot added the integrated Pull request has been integrated label Jan 24, 2025

openjdk bot closed this Jan 24, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 24, 2025

8348203: [JVMCI] Make eager JVMCI initialization observable in the debugger #23219

8348203: [JVMCI] Make eager JVMCI initialization observable in the debugger #23219

Uh oh!

Conversation

simonis commented Jan 21, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 21, 2025

Uh oh!

openjdk bot commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jan 21, 2025

Uh oh!

mlbridge bot commented Jan 21, 2025

Webrevs

Uh oh!

simonis commented Jan 21, 2025

Uh oh!

openjdk bot commented Jan 21, 2025

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

simonis commented Jan 22, 2025

Uh oh!

dholmes-ora commented Jan 23, 2025

Uh oh!

dougxc commented Jan 24, 2025

Uh oh!

simonis commented Jan 24, 2025

Uh oh!

simonis commented Jan 24, 2025

Uh oh!

openjdk bot commented Jan 24, 2025

Uh oh!

openjdk bot commented Jan 24, 2025

Uh oh!

dougxc commented May 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

simonis commented Jan 21, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 21, 2025 •

edited

Loading