Skip to content
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.

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

8346781: [JVMCI] Limit ServiceLoader to class initializers #22869

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* referenced by this might be referenced by {@link ResolvedJavaType} which is kept alive by a
* {@link WeakReference} so we need equivalent reference strength.
*/
abstract class Cleaner extends WeakReference<Object> {
public abstract class Cleaner extends WeakReference<Object> {

/**
* Head of linked list of cleaners.
Expand Down Expand Up @@ -107,7 +107,7 @@ private static synchronized void remove(Cleaner cl) {
/**
* Remove the cleaners whose referents have become weakly reachable.
*/
static void clean() {
public static void clean() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Making this public avoids the need for a method substitution.

Cleaner c = (Cleaner) queue.poll();
boolean oopHandleCleared = false;
while (c != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
package jdk.vm.ci.hotspot;

import static jdk.vm.ci.common.InitTimer.timer;
import static jdk.vm.ci.services.Services.IS_BUILDING_NATIVE_IMAGE;
import static jdk.vm.ci.services.Services.IS_IN_NATIVE_IMAGE;

import java.io.IOException;
Expand Down Expand Up @@ -453,33 +452,26 @@ static void parse(HotSpotJVMCIRuntime runtime) {
}
}

private static HotSpotJVMCIBackendFactory findFactory(String architecture) {
Iterable<HotSpotJVMCIBackendFactory> factories = getHotSpotJVMCIBackendFactories();
assert factories != null : "sanity";
for (HotSpotJVMCIBackendFactory factory : factories) {
if (factory.getArchitecture().equalsIgnoreCase(architecture)) {
return factory;
/**
* The backend factory for the JVMCI shared library.
*/
private static final HotSpotJVMCIBackendFactory backendFactory;
static {
String arch = HotSpotVMConfig.getHostArchitectureName();
HotSpotJVMCIBackendFactory selected = null;
for (HotSpotJVMCIBackendFactory factory : ServiceLoader.load(HotSpotJVMCIBackendFactory.class)) {
if (factory.getArchitecture().equalsIgnoreCase(arch)) {
if (selected != null) {
throw new JVMCIError("Multiple factories available for %s: %s and %s",
arch, selected, factory);
}
selected = factory;
}
}

throw new JVMCIError("No JVMCI runtime available for the %s architecture", architecture);
}

private static volatile List<HotSpotJVMCIBackendFactory> cachedHotSpotJVMCIBackendFactories;

@SuppressFBWarnings(value = "LI_LAZY_INIT_UPDATE_STATIC", justification = "not sure about this")
private static Iterable<HotSpotJVMCIBackendFactory> getHotSpotJVMCIBackendFactories() {
if (IS_IN_NATIVE_IMAGE || cachedHotSpotJVMCIBackendFactories != null) {
return cachedHotSpotJVMCIBackendFactories;
if (selected == null) {
throw new JVMCIError("No JVMCI runtime available for the %s architecture", arch);
}
Iterable<HotSpotJVMCIBackendFactory> result = ServiceLoader.load(HotSpotJVMCIBackendFactory.class, ClassLoader.getSystemClassLoader());
if (IS_BUILDING_NATIVE_IMAGE) {
cachedHotSpotJVMCIBackendFactories = new ArrayList<>();
for (HotSpotJVMCIBackendFactory factory : result) {
cachedHotSpotJVMCIBackendFactories.add(factory);
}
}
return result;
backendFactory = selected;
}

/**
Expand Down Expand Up @@ -587,15 +579,8 @@ private HotSpotJVMCIRuntime() {
// Initialize the Option values.
Option.parse(this);

String hostArchitecture = config.getHostArchitectureName();

HotSpotJVMCIBackendFactory factory;
try (InitTimer t = timer("find factory:", hostArchitecture)) {
factory = findFactory(hostArchitecture);
}

try (InitTimer t = timer("create JVMCI backend:", hostArchitecture)) {
hostBackend = registerBackend(factory.createJVMCIBackend(this, null));
try (InitTimer t = timer("create JVMCI backend:", backendFactory.getArchitecture())) {
hostBackend = registerBackend(backendFactory.createJVMCIBackend(this, null));
}

compilerFactory = HotSpotJVMCICompilerConfig.getCompilerFactory(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ static HotSpotVMConfig config() {
* Gets the host architecture name for the purpose of finding the corresponding
* {@linkplain HotSpotJVMCIBackendFactory backend}.
*/
String getHostArchitectureName() {
static String getHostArchitectureName() {
Architecture arch = Architecture.current();
switch (arch) {
case X64: return "amd64";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
*/
package jdk.vm.ci.services;

import static jdk.vm.ci.services.Services.IS_BUILDING_NATIVE_IMAGE;

import java.util.ArrayList;
import java.util.List;
import java.util.ServiceLoader;
Expand Down Expand Up @@ -71,24 +69,12 @@ protected JVMCIServiceLocator() {
*/
protected abstract <S> S getProvider(Class<S> service);

private static volatile List<JVMCIServiceLocator> cachedLocators;

private static Iterable<JVMCIServiceLocator> getJVMCIServiceLocators() {
Iterable<JVMCIServiceLocator> result = cachedLocators;
if (result != null) {
return result;
}
result = ServiceLoader.load(JVMCIServiceLocator.class, ClassLoader.getSystemClassLoader());
if (IS_BUILDING_NATIVE_IMAGE) {
ArrayList<JVMCIServiceLocator> l = new ArrayList<>();
for (JVMCIServiceLocator locator : result) {
l.add(locator);
}
l.trimToSize();
cachedLocators = l;
return l;
}
return result;
/**
* The available set of locators.
*/
private static final List<JVMCIServiceLocator> locators = new ArrayList<>();
static {
ServiceLoader.load(JVMCIServiceLocator.class).forEach(locators::add);
}

/**
Expand All @@ -106,7 +92,7 @@ public static <S> List<S> getProviders(Class<S> service) {
sm.checkPermission(new JVMCIPermission());
}
List<S> providers = new ArrayList<>();
for (JVMCIServiceLocator access : getJVMCIServiceLocators()) {
for (JVMCIServiceLocator access : locators) {
S provider = access.getProvider(service);
if (provider != null) {
providers.add(provider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@
*/
package jdk.vm.ci.services;

import java.util.ArrayList;
import java.util.Formatter;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.ServiceLoader;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Supplier;
Expand All @@ -44,13 +41,6 @@
*/
public final class Services {

/**
* Guards code that should be run when building an JVMCI shared library but should be excluded
* from (being compiled into) the library. Such code must be directly guarded by an {@code if}
* statement on this field - the guard cannot be behind a method call.
*/
public static final boolean IS_BUILDING_NATIVE_IMAGE = Boolean.parseBoolean(VM.getSavedProperty("jdk.vm.ci.services.aot"));
Copy link
Member Author

Choose a reason for hiding this comment

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

This field is no longer used in JVMCI and I will remove its usages in Graal.


/**
* Guards code that should only be run in a JVMCI shared library. Such code must be directly
* guarded by an {@code if} statement on this field - the guard cannot be behind a method call.
Expand Down Expand Up @@ -131,34 +121,6 @@ public static void initializeJVMCI() {
}
}

private static final Map<Class<?>, List<?>> servicesCache = IS_BUILDING_NATIVE_IMAGE ? new HashMap<>() : null;

@SuppressWarnings("unchecked")
private static <S> Iterable<S> load0(Class<S> service) {
if (IS_IN_NATIVE_IMAGE || IS_BUILDING_NATIVE_IMAGE) {
List<?> list = servicesCache.get(service);
if (list != null) {
return (Iterable<S>) list;
}
if (IS_IN_NATIVE_IMAGE) {
throw new InternalError(String.format("No %s providers found when building native image", service.getName()));
}
}

Iterable<S> providers = ServiceLoader.load(service, ClassLoader.getSystemClassLoader());
if (IS_BUILDING_NATIVE_IMAGE) {
synchronized (servicesCache) {
ArrayList<S> providersList = new ArrayList<>();
for (S provider : providers) {
providersList.add(provider);
}
servicesCache.put(service, providersList);
providers = providersList;
}
}
return providers;
}

/**
* Opens all JVMCI packages to {@code otherModule}.
*/
Expand All @@ -175,57 +137,6 @@ static void openJVMCITo(Module otherModule) {
}
}

/**
* Gets an {@link Iterable} of the JVMCI providers available for a given service.
*
* @throws SecurityException if a security manager is present and it denies <tt>
* {@link RuntimePermission}("jvmci")</tt>
*/
public static <S> Iterable<S> load(Class<S> service) {
Copy link
Member Author

Choose a reason for hiding this comment

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

There are no usages of this method in JVMCI or Graal.

@SuppressWarnings("removal")
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new JVMCIPermission());
}
return load0(service);
}

/**
* Gets the JVMCI provider for a given service for which at most one provider must be available.
*
* @param service the service whose provider is being requested
* @param required specifies if an {@link InternalError} should be thrown if no provider of
* {@code service} is available
* @throws SecurityException if a security manager is present and it denies <tt>
* {@link RuntimePermission}("jvmci")</tt>
*/
public static <S> S loadSingle(Class<S> service, boolean required) {
Copy link
Member Author

Choose a reason for hiding this comment

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

There are no usages of this method in JVMCI or Graal.

@SuppressWarnings("removal")
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new JVMCIPermission());
}
Iterable<S> providers = load0(service);

S singleProvider = null;
for (S provider : providers) {
if (singleProvider != null) {
throw new InternalError(String.format("Multiple %s providers found: %s, %s", service.getName(), singleProvider.getClass().getName(), provider.getClass().getName()));
}
singleProvider = provider;
}
if (singleProvider == null && required) {
String javaHome = Services.getSavedProperty("java.home");
String vmName = Services.getSavedProperty("java.vm.name");
Formatter errorMessage = new Formatter();
errorMessage.format("The VM does not expose required service %s.%n", service.getName());
errorMessage.format("Currently used Java home directory is %s.%n", javaHome);
errorMessage.format("Currently used VM configuration is: %s", vmName);
throw new UnsupportedOperationException(errorMessage.toString());
}
return singleProvider;
}

/**
* Creates a thread-local variable that notifies {@code onThreadTermination} when a thread
* terminates and it has been initialized in the terminating thread (even if it was initialized
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ public class JvmciShutdownEventListener extends JVMCIServiceLocator implements H
public static final String MESSAGE = "Shutdown notified";
public static final String GOT_INTERNAL_ERROR = "Got internal error";

public static void main(String args[]) {
try {
HotSpotJVMCIRuntime.runtime(); // let's trigger that lazy jvmci init
} catch (Error e) {
System.out.println(GOT_INTERNAL_ERROR);
public static class Main {
public static void main(String args[]) {
try {
HotSpotJVMCIRuntime.runtime(); // let's trigger that lazy jvmci init
} catch (Error e) {
System.out.println(GOT_INTERNAL_ERROR);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@
import jdk.test.lib.cli.CommandLineOptionTest;

public class JvmciShutdownEventTest {
private final static String[] MESSAGE = new String[]{
private final static String[] MESSAGE = {
JvmciShutdownEventListener.MESSAGE
};

private final static String[] ERROR_MESSAGE = new String[]{
private final static String[] ERROR_MESSAGE = {
JvmciShutdownEventListener.GOT_INTERNAL_ERROR
};

Expand All @@ -68,15 +68,15 @@ public static void main(String args[]) throws Throwable {
"Unexpected output with +EnableJVMCI", ExitCode.OK,
addTestVMOptions, "-XX:+UnlockExperimentalVMOptions",
"-XX:+EnableJVMCI", "-XX:-UseJVMCICompiler", "-Xbootclasspath/a:.",
JvmciShutdownEventListener.class.getName()
JvmciShutdownEventListener.Main.class.getName()
);

CommandLineOptionTest.verifyJVMStartup(ERROR_MESSAGE, MESSAGE,
"Unexpected exit code with -EnableJVMCI",
"Unexpected output with -EnableJVMCI", ExitCode.OK,
addTestVMOptions, "-XX:+UnlockExperimentalVMOptions",
"-XX:-EnableJVMCI", "-XX:-UseJVMCICompiler", "-Xbootclasspath/a:.",
JvmciShutdownEventListener.class.getName()
JvmciShutdownEventListener.Main.class.getName()
);
}
}