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 all 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: 1 addition & 1 deletion src/hotspot/share/jvmci/jvmciCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ JVMCICompiler::JVMCICompiler() : AbstractCompiler(compiler_jvmci) {

JVMCICompiler* JVMCICompiler::instance(bool require_non_null, TRAPS) {
if (!EnableJVMCI) {
THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), "JVMCI is not enabled")
THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), JVMCI_NOT_ENABLED_ERROR_MESSAGE)
}
if (_instance == nullptr && require_non_null) {
THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), "The JVMCI compiler instance has not been created");
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/jvmci/jvmciRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ JRT_END
JVM_ENTRY_NO_ENV(jobject, JVM_GetJVMCIRuntime(JNIEnv *libjvmciOrHotspotEnv, jclass c))
JVMCIENV_FROM_JNI(thread, libjvmciOrHotspotEnv);
if (!EnableJVMCI) {
JVMCI_THROW_MSG_NULL(InternalError, "JVMCI is not enabled");
JVMCI_THROW_MSG_NULL(InternalError, JVMCI_NOT_ENABLED_ERROR_MESSAGE);
}
JVMCIENV->runtime()->initialize_HotSpotJVMCIRuntime(JVMCI_CHECK_NULL);
JVMCIObject runtime = JVMCIENV->runtime()->get_HotSpotJVMCIRuntime(JVMCI_CHECK_NULL);
Expand All @@ -732,7 +732,7 @@ JVM_END
JVM_ENTRY_NO_ENV(jlong, JVM_ReadSystemPropertiesInfo(JNIEnv *env, jclass c, jintArray offsets_handle))
JVMCIENV_FROM_JNI(thread, env);
if (!EnableJVMCI) {
JVMCI_THROW_MSG_0(InternalError, "JVMCI is not enabled");
JVMCI_THROW_MSG_0(InternalError, JVMCI_NOT_ENABLED_ERROR_MESSAGE);
}
JVMCIPrimitiveArray offsets = JVMCIENV->wrap(offsets_handle);
JVMCIENV->put_int_at(offsets, 0, SystemProperty::next_offset_in_bytes());
Expand Down Expand Up @@ -1515,7 +1515,7 @@ JVM_ENTRY_NO_ENV(void, JVM_RegisterJVMCINatives(JNIEnv *libjvmciOrHotspotEnv, jc
JVMCIENV_FROM_JNI(thread, libjvmciOrHotspotEnv);

if (!EnableJVMCI) {
JVMCI_THROW_MSG(InternalError, "JVMCI is not enabled");
JVMCI_THROW_MSG(InternalError, JVMCI_NOT_ENABLED_ERROR_MESSAGE);
}

JVMCIENV->runtime()->initialize(JVMCIENV);
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/jvmci/jvmciRuntime.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#include "gc/g1/g1CardTable.hpp"
#endif // INCLUDE_G1GC

#define JVMCI_NOT_ENABLED_ERROR_MESSAGE "JVMCI is not enabled. Must specify '-XX:+EnableJVMCI' or '--add-modules=jdk.internal.vm.ci' to the java launcher."

class JVMCIEnv;
class JVMCICompiler;
class JVMCICompileState;
Expand Down
11 changes: 10 additions & 1 deletion src/hotspot/share/jvmci/jvmci_globals.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,16 @@ class fileStream;
constraint) \
\
product(bool, EnableJVMCI, false, EXPERIMENTAL, \
"Enable JVMCI. Defaults to true if UseJVMCICompiler is true.") \
"Enable JVMCI support in the VM. " \
"Defaults to true if UseJVMCICompiler is true or " \
"--add-modules=jdk.internal.vm.ci was specified. " \
"The behavior of --add-modules=jdk.internal.vm.ci is triggered " \
"if any of the following is true: " \
"1. -XX:+EnableJVMCI is set to true on the command line. " \
"2. -XX:+EnableJVMCI is set to true by jdk/internal/vm/options " \
" in the java.base module. " \
"3. EnableJVMCI is defaulted to true by UseJVMCICompiler and " \
" libjvmci is not enabled") \
\
product(bool, UseGraalJIT, false, EXPERIMENTAL, \
"Select the Graal JVMCI compiler. This is an alias for: " \
Expand Down
22 changes: 19 additions & 3 deletions src/hotspot/share/runtime/arguments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ int Arguments::_num_jvm_flags = 0;
char** Arguments::_jvm_args_array = nullptr;
int Arguments::_num_jvm_args = 0;
unsigned int Arguments::_addmods_count = 0;
#if INCLUDE_JVMCI
bool Arguments::_jvmci_module_added = false;
#endif
char* Arguments::_java_command = nullptr;
SystemProperty* Arguments::_system_properties = nullptr;
size_t Arguments::_conservative_max_heap_alignment = 0;
Expand Down Expand Up @@ -1798,9 +1801,9 @@ bool Arguments::check_vm_args_consistency() {
status = CompilerConfig::check_args_consistency(status);
#if INCLUDE_JVMCI
if (status && EnableJVMCI) {
PropertyList_unique_add(&_system_properties, "jdk.internal.vm.ci.enabled", "true",
AddProperty, UnwriteableProperty, InternalProperty);
if (ClassLoader::is_module_observable("jdk.internal.vm.ci")) {
// Add the JVMCI module if not using libjvmci or EnableJVMCI
// was explicitly set on the command line or in the jimage.
if ((!UseJVMCINativeLibrary || FLAG_IS_CMDLINE(EnableJVMCI) || FLAG_IS_JIMAGE_RESOURCE(EnableJVMCI)) && ClassLoader::is_module_observable("jdk.internal.vm.ci") && !_jvmci_module_added) {
if (!create_numbered_module_property("jdk.module.addmods", "jdk.internal.vm.ci", _addmods_count++)) {
return false;
}
Expand Down Expand Up @@ -2247,6 +2250,19 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, JVMFlagOrigin
if (!create_numbered_module_property("jdk.module.addmods", tail, _addmods_count++)) {
return JNI_ENOMEM;
}
#if INCLUDE_JVMCI
if (!_jvmci_module_added) {
const char *jvmci_module = strstr(tail, "jdk.internal.vm.ci");
if (jvmci_module != nullptr) {
char before = *(jvmci_module - 1);
char after = *(jvmci_module + strlen("jdk.internal.vm.ci"));
if ((before == '=' || before == ',') && (after == '\0' || after == ',')) {
FLAG_SET_DEFAULT(EnableJVMCI, true);
_jvmci_module_added = true;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This only works if jdk.internal.vm.ci is specified to --add-modules, it won't set _jvmci_module_added if jdk.internal.vm.ci is resolved because some other module require it. As the module is JDK internal and doesn't export an API then I assume it would be rare-to-never to require it, is that right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. I realized this might be a bit confusing so improved the error message a little: #25240 (comment)
But as you say, it should never be encountered in practice.

#endif
} else if (match_option(option, "--enable-native-access=", &tail)) {
if (!create_numbered_module_property("jdk.module.enable.native.access", tail, enable_native_access_count++)) {
return JNI_ENOMEM;
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/runtime/arguments.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ class Arguments : AllStatic {
static char* _java_command;
// number of unique modules specified in the --add-modules option
static unsigned int _addmods_count;
#if INCLUDE_JVMCI
// was jdk.internal.vm.ci module specified in the --add-modules option?
static bool _jvmci_module_added;
#endif

// Property list
static SystemProperty* _system_properties;
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