From 198457a9663ccefda2846cc5843c8fd7a4910e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20W=C3=B6gerer?= Date: Tue, 1 Jun 2021 16:07:20 +0200 Subject: [PATCH] Replace ImageIncludeBuiltinModules special handling with generic per-module class-init --- .../oracle/svm/driver/MacroOptionHandler.java | 2 - .../com/oracle/svm/driver/NativeImage.java | 9 -- .../hosted/NativeImageClassLoaderSupport.java | 83 +++++++++++++------ ...AbstractNativeImageClassLoaderSupport.java | 26 +++--- .../oracle/svm/hosted/ImageClassLoader.java | 7 +- .../hosted/NativeImageClassLoaderSupport.java | 8 +- .../com/oracle/svm/util/ModuleSupport.java | 50 ++--------- .../com/oracle/svm/util/ModuleSupport.java | 34 +------- truffle/mx.truffle/macro-truffle.properties | 1 - 9 files changed, 91 insertions(+), 129 deletions(-) diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java index 7c7d5dff0085b..1588f62563263 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java @@ -83,8 +83,6 @@ private void applyEnabled(MacroOption.EnabledOption enabledOption, String argume BuildConfiguration config = nativeImage.config; if (!config.useJavaModules()) { enabledOption.forEachPropertyValue(config, "ImageBuilderBootClasspath8", entry -> nativeImage.addImageBuilderBootClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX); - } else { - enabledOption.forEachPropertyValue(config, "ImageIncludeBuiltinModules", entry -> nativeImage.addImageIncludeBuiltinModules(entry), ","); } if (!enabledOption.forEachPropertyValue(config, "ImageBuilderClasspath", entry -> nativeImage.addImageBuilderClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX)) { diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index 7b87d6a6abdf9..a4d11349a6862 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -91,7 +91,6 @@ import com.oracle.svm.driver.MacroOption.EnabledOption; import com.oracle.svm.driver.MacroOption.MacroOptionKind; import com.oracle.svm.driver.MacroOption.Registry; -import com.oracle.svm.hosted.AbstractNativeImageClassLoaderSupport; import com.oracle.svm.hosted.NativeImageGeneratorRunner; import com.oracle.svm.hosted.NativeImageSystemClassLoader; import com.oracle.svm.util.ModuleSupport; @@ -239,7 +238,6 @@ private static String oR(OptionKey option) { private final ArrayList imageBuilderArgs = new ArrayList<>(); private final LinkedHashSet imageBuilderClasspath = new LinkedHashSet<>(); private final LinkedHashSet imageBuilderBootClasspath = new LinkedHashSet<>(); - private final LinkedHashSet imageIncludeBuiltinModules = new LinkedHashSet<>(); private final ArrayList imageBuilderJavaArgs = new ArrayList<>(); private final LinkedHashSet imageClasspath = new LinkedHashSet<>(); private final LinkedHashSet imageProvidedClasspath = new LinkedHashSet<>(); @@ -1193,9 +1191,6 @@ private int completeImageBuild() { // The following two are for backwards compatibility reasons. They should be removed. imageBuilderJavaArgs.add("-Djdk.internal.lambda.eagerlyInitialize=false"); imageBuilderJavaArgs.add("-Djava.lang.invoke.InnerClassLambdaMetafactory.initializeLambdas=false"); - if (!imageIncludeBuiltinModules.isEmpty()) { - imageBuilderJavaArgs.add("-D" + AbstractNativeImageClassLoaderSupport.PROPERTY_IMAGEINCLUDEBUILTINMODULES + "=" + String.join(",", imageIncludeBuiltinModules)); - } /* After JavaArgs consolidation add the user provided JavaArgs */ boolean afterOption = false; @@ -1622,10 +1617,6 @@ void addImageBuilderBootClasspath(Path classpath) { imageBuilderBootClasspath.add(canonicalize(classpath)); } - public void addImageIncludeBuiltinModules(String moduleName) { - imageIncludeBuiltinModules.add(moduleName); - } - void addImageBuilderJavaArgs(String... javaArgs) { addImageBuilderJavaArgs(Arrays.asList(javaArgs)); } diff --git a/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java b/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java index bed2e489d5697..36bff9aeab781 100644 --- a/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java +++ b/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java @@ -25,15 +25,20 @@ package com.oracle.svm.hosted; import java.io.File; +import java.io.IOException; import java.lang.module.Configuration; import java.lang.module.ModuleDescriptor; import java.lang.module.ModuleFinder; +import java.lang.module.ModuleReader; +import java.lang.module.ModuleReference; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -48,7 +53,6 @@ import com.oracle.svm.core.option.SubstrateOptionsParser; import com.oracle.svm.core.util.UserError; import com.oracle.svm.core.util.VMError; -import com.oracle.svm.util.ModuleSupport; import jdk.internal.module.Modules; @@ -59,10 +63,13 @@ public class NativeImageClassLoaderSupport extends AbstractNativeImageClassLoade private final ClassLoader classLoader; private final ModuleLayer moduleLayerForImageBuild; + private final Map> packageToModuleNames; NativeImageClassLoaderSupport(ClassLoader defaultSystemClassLoader, String[] classpath, String[] modulePath) { super(defaultSystemClassLoader, classpath); + packageToModuleNames = new HashMap<>(); + imagemp = Arrays.stream(modulePath).map(Paths::get).collect(Collectors.toUnmodifiableList()); buildmp = Arrays.stream(System.getProperty("jdk.module.path", "").split(File.pathSeparator)).map(Paths::get).collect(Collectors.toUnmodifiableList()); @@ -143,6 +150,11 @@ public Optional findModule(String moduleName) { return moduleLayerForImageBuild.findModule(moduleName); } + @Override + Set findModuleNamesForPackage(String packageName) { + return packageToModuleNames.getOrDefault(packageName, Collections.emptySet()); + } + @Override void processAddExportsAndAddOpens(OptionValues parsedHostedOptions) { LocatableMultiOptionValue.Strings addExports = NativeImageClassLoaderOptions.AddExports.getValue(parsedHostedOptions); @@ -254,52 +266,69 @@ ClassLoader getClassLoader() { return classLoader; } - private static class ClassInitWithModules extends ClassInit { + private class ClassInitWithModules extends ClassInit { - ClassInitWithModules(ForkJoinPool executor, ImageClassLoader imageClassLoader, AbstractNativeImageClassLoaderSupport nativeImageClassLoader) { - super(executor, imageClassLoader, nativeImageClassLoader); + ClassInitWithModules(ForkJoinPool executor, ImageClassLoader imageClassLoader) { + super(executor, imageClassLoader); } @Override protected void init() { - Set modules = new HashSet<>(); - modules.add("jdk.internal.vm.ci"); - - addOptionalModule(modules, "org.graalvm.sdk"); - addOptionalModule(modules, "jdk.internal.vm.compiler"); - addOptionalModule(modules, "com.oracle.graal.graal_enterprise"); - - String includeModulesStr = System.getProperty(PROPERTY_IMAGEINCLUDEBUILTINMODULES); - if (includeModulesStr != null) { - modules.addAll(Arrays.asList(includeModulesStr.split(","))); - } - - for (String moduleResource : ModuleSupport.getSystemModuleResources(modules)) { - handleClassInModuleResource(moduleResource); - } - - for (String moduleResource : ModuleSupport.getModuleResources(nativeImageClassLoader.modulepath())) { - handleClassInModuleResource(moduleResource); + ModuleFinder finder = ModuleFinder.compose(ModuleFinder.ofSystem(), ModuleFinder.of(modulepath().toArray(Path[]::new))); + for (ModuleReference moduleReference : finder.findAll()) { + try (ModuleReader moduleReader = moduleReference.open()) { + moduleReader.list().forEach(moduleResource -> { + handleModuleResource(moduleReference.descriptor().name(), moduleResource); + }); + } catch (IOException e) { + throw new RuntimeException("Unable get list of resources in module" + moduleReference.descriptor().name(), e); + } } super.init(); } - private void handleClassInModuleResource(String moduleResource) { + private void handleModuleResource(String moduleName, String moduleResource) { + if (!moduleResource.startsWith("META-INF/")) { + addToPackageNameModules(moduleName, moduleResource); + } if (moduleResource.endsWith(CLASS_EXTENSION)) { executor.execute(() -> handleClassFileName(classFileWithoutSuffix(moduleResource), '/')); } } - private static void addOptionalModule(Set modules, String name) { - if (ModuleSupport.hasSystemModule(name)) { - modules.add(name); + private void addToPackageNameModules(String moduleName, String moduleResource) { + int packageSep = moduleResource.lastIndexOf('/'); + String packageName = packageSep > 0 ? moduleResource.substring(0, packageSep).replace('/', '.') : ""; + Set prevValue = packageToModuleNames.get(packageName); + if (prevValue == null) { + /* Mostly packageName is only used in a single module */ + packageToModuleNames.put(packageName, Collections.singleton(moduleName)); + } else if (prevValue.size() == 1) { + /* Transition to HashSet - happens rarely */ + HashSet newValue = new HashSet<>(); + newValue.add(prevValue.iterator().next()); + newValue.add(moduleName); + packageToModuleNames.put(packageName, newValue); + } else if (prevValue.size() > 1) { + /* Add to exiting HashSet - happens rarely */ + prevValue.add(moduleName); } } } @Override public void initAllClasses(ForkJoinPool executor, ImageClassLoader imageClassLoader) { - new ClassInitWithModules(executor, imageClassLoader, this).init(); + new ClassInitWithModules(executor, imageClassLoader).init(); + // dumpPackageNameModulesMapping(); + } + + public void dumpPackageNameModulesMapping() { + packageToModuleNames.entrySet().stream() + .sorted(Map.Entry.comparingByKey()) + .map(e -> e.getKey() + " -> " + e.getValue().stream() + .map(mn -> (findModule(mn).isEmpty() ? "*" : "") + mn) + .collect(Collectors.joining(", "))) + .forEach(System.out::println); } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java index a891896cbb9d0..0535e8896f7d2 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java @@ -65,12 +65,6 @@ public abstract class AbstractNativeImageClassLoaderSupport { - /* - * This cannot be a HostedOption because all Subclasses of OptionDescriptors from inside builtin - * modules need to be initialized prior to option parsing so that they can be found. - */ - public static final String PROPERTY_IMAGEINCLUDEBUILTINMODULES = "substratevm.ImageIncludeBuiltinModules"; - final List imagecp; private final List buildcp; @@ -113,6 +107,8 @@ ClassLoader getClassLoader() { abstract Optional findModule(String moduleName); + abstract Set findModuleNamesForPackage(String packageName); + abstract void processAddExportsAndAddOpens(OptionValues parsedHostedOptions); abstract void initAllClasses(ForkJoinPool executor, ImageClassLoader imageClassLoader); @@ -154,25 +150,23 @@ static Path urlToPath(URL url) { } } - protected static class ClassInit { + protected class ClassInit { protected final ForkJoinPool executor; protected final ImageClassLoader imageClassLoader; - protected final AbstractNativeImageClassLoaderSupport nativeImageClassLoader; - ClassInit(ForkJoinPool executor, ImageClassLoader imageClassLoader, AbstractNativeImageClassLoaderSupport nativeImageClassLoader) { + ClassInit(ForkJoinPool executor, ImageClassLoader imageClassLoader) { this.executor = executor; this.imageClassLoader = imageClassLoader; - this.nativeImageClassLoader = nativeImageClassLoader; } protected void init() { - Set uniquePaths = new TreeSet<>(Comparator.comparing(ClassInit::toRealPath)); - uniquePaths.addAll(nativeImageClassLoader.classpath()); + Set uniquePaths = new TreeSet<>(Comparator.comparing(this::toRealPath)); + uniquePaths.addAll(classpath()); uniquePaths.parallelStream().forEach(path -> loadClassesFromPath(path)); } - private static Path toRealPath(Path p) { + private Path toRealPath(Path p) { try { return p.toRealPath(); } catch (IOException e) { @@ -180,9 +174,9 @@ private static Path toRealPath(Path p) { } } - private static final Set excludeDirectories = getExcludeDirectories(); + private final Set excludeDirectories = getExcludeDirectories(); - private static Set getExcludeDirectories() { + private Set getExcludeDirectories() { Path root = Paths.get("/"); return Stream.of("dev", "sys", "proc", "etc", "var", "tmp", "boot", "lost+found") .map(root::resolve).collect(Collectors.toSet()); @@ -278,7 +272,7 @@ private String unversionedFileName(String fileName) { } } - static String classFileWithoutSuffix(String result) { + String classFileWithoutSuffix(String result) { return result.substring(0, result.length() - CLASS_EXTENSION.length()); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java index 322eea7660a9a..ebf229f8edaef 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java @@ -38,6 +38,7 @@ import java.util.Enumeration; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -418,10 +419,14 @@ public Class loadClassFromModule(Object module, String className) throws Clas return classLoaderSupport.loadClassFromModule(module, className); } - public Optional findModule(String moduleName) { + public Optional findModule(String moduleName) { return classLoaderSupport.findModule(moduleName); } + public Set findModuleNamesForPackage(String packageName) { + return classLoaderSupport.findModuleNamesForPackage(packageName); + } + public void processAddExportsAndAddOpens(OptionValues parsedHostedOptions) { classLoaderSupport.processAddExportsAndAddOpens(parsedHostedOptions); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java index a9f209bcbb767..4460ae067046f 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java @@ -28,6 +28,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ForkJoinPool; import org.graalvm.compiler.options.OptionValues; @@ -53,6 +54,11 @@ public Optional findModule(String moduleName) { return Optional.empty(); } + @Override + public Set findModuleNamesForPackage(String packageName) { + return Collections.emptySet(); + } + @Override void processAddExportsAndAddOpens(OptionValues parsedHostedOptions) { /* Nothing to do for Java 8 */ @@ -69,6 +75,6 @@ Class loadClassFromModule(Object module, String className) throws ClassNotFou @Override public void initAllClasses(ForkJoinPool executor, ImageClassLoader imageClassLoader) { - new ClassInit(executor, imageClassLoader, this).init(); + new ClassInit(executor, imageClassLoader).init(); } } diff --git a/substratevm/src/com.oracle.svm.util.jdk11/src/com/oracle/svm/util/ModuleSupport.java b/substratevm/src/com.oracle.svm.util.jdk11/src/com/oracle/svm/util/ModuleSupport.java index 546fb833ebcc7..e93fd538b898f 100644 --- a/substratevm/src/com.oracle.svm.util.jdk11/src/com/oracle/svm/util/ModuleSupport.java +++ b/substratevm/src/com.oracle.svm.util.jdk11/src/com/oracle/svm/util/ModuleSupport.java @@ -26,13 +26,9 @@ import java.io.IOException; import java.io.InputStream; -import java.lang.module.ModuleFinder; import java.lang.module.ModuleReader; import java.lang.module.ModuleReference; import java.lang.module.ResolvedModule; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Locale; import java.util.MissingResourceException; @@ -44,8 +40,12 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import org.graalvm.nativeimage.Platform; +import org.graalvm.nativeimage.Platforms; + import jdk.internal.module.Modules; +@Platforms(Platform.HOSTED_ONLY.class) public final class ModuleSupport { private ModuleSupport() { } @@ -82,46 +82,14 @@ private static ResourceBundle getResourceBundleFallback(String bundleName, Local return ResourceBundle.getBundle(bundleName, locale, loader); } - public static boolean hasSystemModule(String moduleName) { - return ModuleFinder.ofSystem().find(moduleName).isPresent(); - } - - public static List getModuleResources(Collection modulePath) { - ArrayList result = new ArrayList<>(); - for (ModuleReference moduleReference : ModuleFinder.of(modulePath.toArray(Path[]::new)).findAll()) { - try (ModuleReader moduleReader = moduleReference.open()) { - result.addAll(moduleReader.list().collect(Collectors.toList())); - } catch (IOException e) { - throw new RuntimeException("Unable get list of resources in module" + moduleReference.descriptor().name(), e); - } - } - return result; - } - - public static List getSystemModuleResources(Collection names) { - List result = new ArrayList<>(); - for (String name : names) { - Optional moduleReference = ModuleFinder.ofSystem().find(name); - if (moduleReference.isEmpty()) { - throw new RuntimeException("Unable find ModuleReference for module " + name); - } - try (ModuleReader moduleReader = moduleReference.get().open()) { - result.addAll(moduleReader.list().collect(Collectors.toList())); - } catch (IOException e) { - throw new RuntimeException("Unable get list of resources in module" + name, e); - } - } - return result; - } - public static void openModuleByClass(Class declaringClass, Class accessingClass) { Module declaringModule = declaringClass.getModule(); String packageName = declaringClass.getPackageName(); - Module accessingModule = accessingClass == null ? null : accessingClass.getModule(); - if (accessingModule != null && accessingModule.isNamed()) { - if (!declaringModule.isOpen(packageName, accessingModule)) { - Modules.addOpens(declaringModule, packageName, accessingModule); - } + if (accessingClass != null ? declaringModule.isOpen(packageName, accessingClass.getModule()) : declaringModule.isOpen(packageName)) { + return; + } + if (accessingClass != null && accessingClass.getModule().isNamed()) { + Modules.addOpens(declaringModule, packageName, accessingClass.getModule()); } else { Modules.addOpensToAllUnnamed(declaringModule, packageName); } diff --git a/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/ModuleSupport.java b/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/ModuleSupport.java index d8b957358cfcc..59d5b3e89e26b 100644 --- a/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/ModuleSupport.java +++ b/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/ModuleSupport.java @@ -26,10 +26,6 @@ import java.io.IOException; import java.io.InputStream; -import java.nio.file.Path; -import java.util.Collection; -import java.util.Collections; -import java.util.List; import java.util.Locale; import java.util.NoSuchElementException; import java.util.ResourceBundle; @@ -37,7 +33,10 @@ import java.util.function.Predicate; import org.graalvm.compiler.serviceprovider.JavaVersionUtil; +import org.graalvm.nativeimage.Platform; +import org.graalvm.nativeimage.Platforms; +@Platforms(Platform.HOSTED_ONLY.class) public final class ModuleSupport { private ModuleSupport() { } @@ -46,33 +45,6 @@ public static ResourceBundle getResourceBundle(String bundleName, Locale locale, return ResourceBundle.getBundle(bundleName, locale, loader); } - /** - * Checks if the Java run-time image contains a module with the given name. - */ - @SuppressWarnings("unused") - public static boolean hasSystemModule(String moduleName) { - /* Nothing to do in JDK 8 version. JDK 11 version provides a proper implementation. */ - assert JavaVersionUtil.JAVA_SPEC <= 8; - return false; - } - - /** - * Gets all resources in the modules named by {@code modules} from the Java runtime image. - */ - @SuppressWarnings("unused") - public static List getModuleResources(Collection modulePath) { - /* Nothing to do in JDK 8 version. JDK 11 version provides a proper implementation. */ - assert JavaVersionUtil.JAVA_SPEC <= 8; - return Collections.emptyList(); - } - - @SuppressWarnings("unused") - public static List getSystemModuleResources(Collection names) { - /* Nothing to do in JDK 8 version. JDK 11 version provides a proper implementation. */ - assert JavaVersionUtil.JAVA_SPEC <= 8; - return Collections.emptyList(); - } - /** * Add the proper module opening to allow accesses from accessingClass to declaringClass. */ diff --git a/truffle/mx.truffle/macro-truffle.properties b/truffle/mx.truffle/macro-truffle.properties index 0c4eae8e808d6..8ecf9e01d8d45 100644 --- a/truffle/mx.truffle/macro-truffle.properties +++ b/truffle/mx.truffle/macro-truffle.properties @@ -1,6 +1,5 @@ # This file contains support for building truffle images ImageBuilderBootClasspath8 = ${.}/../../../truffle/truffle-api.jar -ImageIncludeBuiltinModules = org.graalvm.truffle Args = -H:Features=com.oracle.svm.truffle.TruffleFeature,org.graalvm.home.HomeFinderFeature \ -H:MaxRuntimeCompileMethods=2000 \