Skip to content

8345826: Do not automatically resolve jdk.internal.vm.ci when libgraal is used #25240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

8345826: Do not automatically resolve jdk.internal.vm.ci when libgraal is used #25240

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
171703a
--add-modules=jdk.internal.vm.ci implies -XX:+EnableJVMCI
dougxc May 13, 2025
0e8773e
added comment in check_vm_args_consistency
dougxc May 15, 2025
3436033
resolve jdk.internal.vm.ci if +EnableJVMCI and -UseJVMCINativeLibrary
dougxc May 16, 2025
cc49ed0
fix TestHotSpotJVMCIRuntime
dougxc May 16, 2025
7b30238
removed use of jdk.internal.vm.ci.enabled property
dougxc May 16, 2025
3cdef58
fixed comment
dougxc May 16, 2025
1fe56b4
improved error message
dougxc May 16, 2025
cd615ef
JAVA_NOT_ENABLED_ERROR_MESSAGE -> JVMCI_NOT_ENABLED_ERROR_MESSAGE
dougxc May 16, 2025
d9223af
load the JVMCI module if +EnableJVMCI is set on the command line
dougxc May 16, 2025
196425f
load the JVMCI module if +EnableJVMCI is set in the jimage
dougxc May 18, 2025
b74077f
swapped order of recommended options in error message
dougxc May 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/hotspot/share/runtime/arguments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1810,8 +1810,6 @@ bool Arguments::check_vm_args_consistency() {
return false;
}
}
PropertyList_unique_add(&_system_properties, "jdk.internal.vm.ci.enabled", "true",
AddProperty, UnwriteableProperty, InternalProperty);
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/arguments.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ class Arguments : AllStatic {
static char* _java_command;
// number of unique modules specified in the --add-modules option
static unsigned int _addmods_count;
// number of unique modules specified in the --add-modules option
#if INCLUDE_JVMCI
// was jdk.internal.vm.ci module specified in the --add-modules option?
static bool _jvmci_module_added;
#endif

Expand Down
2 changes: 0 additions & 2 deletions src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/JVMCIServiceLocator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ private JVMCIServiceLocator(Void ignore) {
*/
protected JVMCIServiceLocator() {
this(checkPermission());
Services.checkJVMCIEnabled();
Services.openJVMCITo(getClass().getModule());
}

Expand All @@ -85,7 +84,6 @@ protected JVMCIServiceLocator() {
* {@link JVMCIPermission}
*/
public static <S> List<S> getProviders(Class<S> service) {
Services.checkJVMCIEnabled();
@SuppressWarnings("removal")
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
Expand Down
13 changes: 0 additions & 13 deletions src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/Services.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,6 @@ private Services() {
*/
private static volatile Map<String, String> savedProperties;

static final boolean JVMCI_ENABLED = Boolean.parseBoolean(VM.getSavedProperties().get("jdk.internal.vm.ci.enabled"));

/**
* Checks that JVMCI is enabled in the VM and throws an error if it isn't.
*/
static void checkJVMCIEnabled() {
if (!JVMCI_ENABLED) {
throw new Error("The EnableJVMCI VM option must be true (i.e., -XX:+EnableJVMCI) to use JVMCI");
}
}

/**
* Gets an unmodifiable copy of the system properties as of VM startup.
*
Expand All @@ -84,7 +73,6 @@ static void checkJVMCIEnabled() {
* on the command line are ignored.
*/
public static Map<String, String> getSavedProperties() {
checkJVMCIEnabled();
if (savedProperties == null) {
synchronized (Services.class) {
if (savedProperties == null) {
Expand Down Expand Up @@ -113,7 +101,6 @@ public static String getSavedProperty(String name) {
* Causes the JVMCI subsystem to be initialized if it isn't already initialized.
*/
public static void initializeJVMCI() {
checkJVMCIEnabled();
try {
Class.forName("jdk.vm.ci.runtime.JVMCI");
} catch (ClassNotFoundException e) {
Expand Down
1 change: 1 addition & 0 deletions ...iler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestHotSpotJVMCIRuntime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ public void jniEnomemTest() throws Exception {
ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(
"-XX:+UnlockExperimentalVMOptions",
"-XX:+EnableJVMCI",
"--add-modules=jdk.internal.vm.ci",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stared at this for a while to understand why passing this option was required. It's a bit confusing that explicitly passing -XX:+EnableJVMCI has different effects based on the value of UseJVMCINativeLibrary. I think that if EnableJVMCI is passed on the command line then it should add the module even if libgraal is in use. So something like:
if ((!UseJVMCINativeLibrary || FLAG_IS_CMDLINE(EnableJVMCI) && ClassLoader::is_module_observable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware FLAG_IS_CMDLINE can be used for altering the semantics of a flag but there seems to be at least one precedent for it with UseCompactObjectHeaders. This is quite nice as now nothing needs to change for Truffle users in terms of enabling the Truffle optimized runtime (cc @chumer).

"-XX:-UseJVMCICompiler",
"-XX:+UseJVMCINativeLibrary",
"-Dtest.jvmci.forceEnomemOnLibjvmciInit=true",
Expand Down