Skip to content

Commit 198457a

Browse files
olpawolpaw
committed
Replace ImageIncludeBuiltinModules special handling with generic per-module class-init
1 parent bd34f5a commit 198457a

File tree

9 files changed

+91
-129
lines changed

9 files changed

+91
-129
lines changed

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MacroOptionHandler.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ private void applyEnabled(MacroOption.EnabledOption enabledOption, String argume
8383
BuildConfiguration config = nativeImage.config;
8484
if (!config.useJavaModules()) {
8585
enabledOption.forEachPropertyValue(config, "ImageBuilderBootClasspath8", entry -> nativeImage.addImageBuilderBootClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX);
86-
} else {
87-
enabledOption.forEachPropertyValue(config, "ImageIncludeBuiltinModules", entry -> nativeImage.addImageIncludeBuiltinModules(entry), ",");
8886
}
8987

9088
if (!enabledOption.forEachPropertyValue(config, "ImageBuilderClasspath", entry -> nativeImage.addImageBuilderClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX)) {

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@
9191
import com.oracle.svm.driver.MacroOption.EnabledOption;
9292
import com.oracle.svm.driver.MacroOption.MacroOptionKind;
9393
import com.oracle.svm.driver.MacroOption.Registry;
94-
import com.oracle.svm.hosted.AbstractNativeImageClassLoaderSupport;
9594
import com.oracle.svm.hosted.NativeImageGeneratorRunner;
9695
import com.oracle.svm.hosted.NativeImageSystemClassLoader;
9796
import com.oracle.svm.util.ModuleSupport;
@@ -239,7 +238,6 @@ private static <T> String oR(OptionKey<T> option) {
239238
private final ArrayList<String> imageBuilderArgs = new ArrayList<>();
240239
private final LinkedHashSet<Path> imageBuilderClasspath = new LinkedHashSet<>();
241240
private final LinkedHashSet<Path> imageBuilderBootClasspath = new LinkedHashSet<>();
242-
private final LinkedHashSet<String> imageIncludeBuiltinModules = new LinkedHashSet<>();
243241
private final ArrayList<String> imageBuilderJavaArgs = new ArrayList<>();
244242
private final LinkedHashSet<Path> imageClasspath = new LinkedHashSet<>();
245243
private final LinkedHashSet<Path> imageProvidedClasspath = new LinkedHashSet<>();
@@ -1193,9 +1191,6 @@ private int completeImageBuild() {
11931191
// The following two are for backwards compatibility reasons. They should be removed.
11941192
imageBuilderJavaArgs.add("-Djdk.internal.lambda.eagerlyInitialize=false");
11951193
imageBuilderJavaArgs.add("-Djava.lang.invoke.InnerClassLambdaMetafactory.initializeLambdas=false");
1196-
if (!imageIncludeBuiltinModules.isEmpty()) {
1197-
imageBuilderJavaArgs.add("-D" + AbstractNativeImageClassLoaderSupport.PROPERTY_IMAGEINCLUDEBUILTINMODULES + "=" + String.join(",", imageIncludeBuiltinModules));
1198-
}
11991194

12001195
/* After JavaArgs consolidation add the user provided JavaArgs */
12011196
boolean afterOption = false;
@@ -1622,10 +1617,6 @@ void addImageBuilderBootClasspath(Path classpath) {
16221617
imageBuilderBootClasspath.add(canonicalize(classpath));
16231618
}
16241619

1625-
public void addImageIncludeBuiltinModules(String moduleName) {
1626-
imageIncludeBuiltinModules.add(moduleName);
1627-
}
1628-
16291620
void addImageBuilderJavaArgs(String... javaArgs) {
16301621
addImageBuilderJavaArgs(Arrays.asList(javaArgs));
16311622
}

‎substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java‎

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,20 @@
2525
package com.oracle.svm.hosted;
2626

2727
import java.io.File;
28+
import java.io.IOException;
2829
import java.lang.module.Configuration;
2930
import java.lang.module.ModuleDescriptor;
3031
import java.lang.module.ModuleFinder;
32+
import java.lang.module.ModuleReader;
33+
import java.lang.module.ModuleReference;
3134
import java.nio.file.Path;
3235
import java.nio.file.Paths;
3336
import java.util.Arrays;
3437
import java.util.Collections;
38+
import java.util.HashMap;
3539
import java.util.HashSet;
3640
import java.util.List;
41+
import java.util.Map;
3742
import java.util.Objects;
3843
import java.util.Optional;
3944
import java.util.Set;
@@ -48,7 +53,6 @@
4853
import com.oracle.svm.core.option.SubstrateOptionsParser;
4954
import com.oracle.svm.core.util.UserError;
5055
import com.oracle.svm.core.util.VMError;
51-
import com.oracle.svm.util.ModuleSupport;
5256

5357
import jdk.internal.module.Modules;
5458

@@ -59,10 +63,13 @@ public class NativeImageClassLoaderSupport extends AbstractNativeImageClassLoade
5963

6064
private final ClassLoader classLoader;
6165
private final ModuleLayer moduleLayerForImageBuild;
66+
private final Map<String, Set<String>> packageToModuleNames;
6267

6368
NativeImageClassLoaderSupport(ClassLoader defaultSystemClassLoader, String[] classpath, String[] modulePath) {
6469
super(defaultSystemClassLoader, classpath);
6570

71+
packageToModuleNames = new HashMap<>();
72+
6673
imagemp = Arrays.stream(modulePath).map(Paths::get).collect(Collectors.toUnmodifiableList());
6774
buildmp = Arrays.stream(System.getProperty("jdk.module.path", "").split(File.pathSeparator)).map(Paths::get).collect(Collectors.toUnmodifiableList());
6875

@@ -143,6 +150,11 @@ public Optional<Module> findModule(String moduleName) {
143150
return moduleLayerForImageBuild.findModule(moduleName);
144151
}
145152

153+
@Override
154+
Set<String> findModuleNamesForPackage(String packageName) {
155+
return packageToModuleNames.getOrDefault(packageName, Collections.emptySet());
156+
}
157+
146158
@Override
147159
void processAddExportsAndAddOpens(OptionValues parsedHostedOptions) {
148160
LocatableMultiOptionValue.Strings addExports = NativeImageClassLoaderOptions.AddExports.getValue(parsedHostedOptions);
@@ -254,52 +266,69 @@ ClassLoader getClassLoader() {
254266
return classLoader;
255267
}
256268

257-
private static class ClassInitWithModules extends ClassInit {
269+
private class ClassInitWithModules extends ClassInit {
258270

259-
ClassInitWithModules(ForkJoinPool executor, ImageClassLoader imageClassLoader, AbstractNativeImageClassLoaderSupport nativeImageClassLoader) {
260-
super(executor, imageClassLoader, nativeImageClassLoader);
271+
ClassInitWithModules(ForkJoinPool executor, ImageClassLoader imageClassLoader) {
272+
super(executor, imageClassLoader);
261273
}
262274

263275
@Override
264276
protected void init() {
265-
Set<String> modules = new HashSet<>();
266-
modules.add("jdk.internal.vm.ci");
267-
268-
addOptionalModule(modules, "org.graalvm.sdk");
269-
addOptionalModule(modules, "jdk.internal.vm.compiler");
270-
addOptionalModule(modules, "com.oracle.graal.graal_enterprise");
271-
272-
String includeModulesStr = System.getProperty(PROPERTY_IMAGEINCLUDEBUILTINMODULES);
273-
if (includeModulesStr != null) {
274-
modules.addAll(Arrays.asList(includeModulesStr.split(",")));
275-
}
276-
277-
for (String moduleResource : ModuleSupport.getSystemModuleResources(modules)) {
278-
handleClassInModuleResource(moduleResource);
279-
}
280-
281-
for (String moduleResource : ModuleSupport.getModuleResources(nativeImageClassLoader.modulepath())) {
282-
handleClassInModuleResource(moduleResource);
277+
ModuleFinder finder = ModuleFinder.compose(ModuleFinder.ofSystem(), ModuleFinder.of(modulepath().toArray(Path[]::new)));
278+
for (ModuleReference moduleReference : finder.findAll()) {
279+
try (ModuleReader moduleReader = moduleReference.open()) {
280+
moduleReader.list().forEach(moduleResource -> {
281+
handleModuleResource(moduleReference.descriptor().name(), moduleResource);
282+
});
283+
} catch (IOException e) {
284+
throw new RuntimeException("Unable get list of resources in module" + moduleReference.descriptor().name(), e);
285+
}
283286
}
284287

285288
super.init();
286289
}
287290

288-
private void handleClassInModuleResource(String moduleResource) {
291+
private void handleModuleResource(String moduleName, String moduleResource) {
292+
if (!moduleResource.startsWith("META-INF/")) {
293+
addToPackageNameModules(moduleName, moduleResource);
294+
}
289295
if (moduleResource.endsWith(CLASS_EXTENSION)) {
290296
executor.execute(() -> handleClassFileName(classFileWithoutSuffix(moduleResource), '/'));
291297
}
292298
}
293299

294-
private static void addOptionalModule(Set<String> modules, String name) {
295-
if (ModuleSupport.hasSystemModule(name)) {
296-
modules.add(name);
300+
private void addToPackageNameModules(String moduleName, String moduleResource) {
301+
int packageSep = moduleResource.lastIndexOf('/');
302+
String packageName = packageSep > 0 ? moduleResource.substring(0, packageSep).replace('/', '.') : "";
303+
Set<String> prevValue = packageToModuleNames.get(packageName);
304+
if (prevValue == null) {
305+
/* Mostly packageName is only used in a single module */
306+
packageToModuleNames.put(packageName, Collections.singleton(moduleName));
307+
} else if (prevValue.size() == 1) {
308+
/* Transition to HashSet - happens rarely */
309+
HashSet<String> newValue = new HashSet<>();
310+
newValue.add(prevValue.iterator().next());
311+
newValue.add(moduleName);
312+
packageToModuleNames.put(packageName, newValue);
313+
} else if (prevValue.size() > 1) {
314+
/* Add to exiting HashSet - happens rarely */
315+
prevValue.add(moduleName);
297316
}
298317
}
299318
}
300319

301320
@Override
302321
public void initAllClasses(ForkJoinPool executor, ImageClassLoader imageClassLoader) {
303-
new ClassInitWithModules(executor, imageClassLoader, this).init();
322+
new ClassInitWithModules(executor, imageClassLoader).init();
323+
// dumpPackageNameModulesMapping();
324+
}
325+
326+
public void dumpPackageNameModulesMapping() {
327+
packageToModuleNames.entrySet().stream()
328+
.sorted(Map.Entry.comparingByKey())
329+
.map(e -> e.getKey() + " -> " + e.getValue().stream()
330+
.map(mn -> (findModule(mn).isEmpty() ? "*" : "") + mn)
331+
.collect(Collectors.joining(", ")))
332+
.forEach(System.out::println);
304333
}
305334
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,6 @@
6565

6666
public abstract class AbstractNativeImageClassLoaderSupport {
6767

68-
/*
69-
* This cannot be a HostedOption because all Subclasses of OptionDescriptors from inside builtin
70-
* modules need to be initialized prior to option parsing so that they can be found.
71-
*/
72-
public static final String PROPERTY_IMAGEINCLUDEBUILTINMODULES = "substratevm.ImageIncludeBuiltinModules";
73-
7468
final List<Path> imagecp;
7569
private final List<Path> buildcp;
7670

@@ -113,6 +107,8 @@ ClassLoader getClassLoader() {
113107

114108
abstract Optional<? extends Object> findModule(String moduleName);
115109

110+
abstract Set<String> findModuleNamesForPackage(String packageName);
111+
116112
abstract void processAddExportsAndAddOpens(OptionValues parsedHostedOptions);
117113

118114
abstract void initAllClasses(ForkJoinPool executor, ImageClassLoader imageClassLoader);
@@ -154,35 +150,33 @@ static Path urlToPath(URL url) {
154150
}
155151
}
156152

157-
protected static class ClassInit {
153+
protected class ClassInit {
158154

159155
protected final ForkJoinPool executor;
160156
protected final ImageClassLoader imageClassLoader;
161-
protected final AbstractNativeImageClassLoaderSupport nativeImageClassLoader;
162157

163-
ClassInit(ForkJoinPool executor, ImageClassLoader imageClassLoader, AbstractNativeImageClassLoaderSupport nativeImageClassLoader) {
158+
ClassInit(ForkJoinPool executor, ImageClassLoader imageClassLoader) {
164159
this.executor = executor;
165160
this.imageClassLoader = imageClassLoader;
166-
this.nativeImageClassLoader = nativeImageClassLoader;
167161
}
168162

169163
protected void init() {
170-
Set<Path> uniquePaths = new TreeSet<>(Comparator.comparing(ClassInit::toRealPath));
171-
uniquePaths.addAll(nativeImageClassLoader.classpath());
164+
Set<Path> uniquePaths = new TreeSet<>(Comparator.comparing(this::toRealPath));
165+
uniquePaths.addAll(classpath());
172166
uniquePaths.parallelStream().forEach(path -> loadClassesFromPath(path));
173167
}
174168

175-
private static Path toRealPath(Path p) {
169+
private Path toRealPath(Path p) {
176170
try {
177171
return p.toRealPath();
178172
} catch (IOException e) {
179173
throw VMError.shouldNotReachHere("Path.toRealPath failed for " + p, e);
180174
}
181175
}
182176

183-
private static final Set<Path> excludeDirectories = getExcludeDirectories();
177+
private final Set<Path> excludeDirectories = getExcludeDirectories();
184178

185-
private static Set<Path> getExcludeDirectories() {
179+
private Set<Path> getExcludeDirectories() {
186180
Path root = Paths.get("/");
187181
return Stream.of("dev", "sys", "proc", "etc", "var", "tmp", "boot", "lost+found")
188182
.map(root::resolve).collect(Collectors.toSet());
@@ -278,7 +272,7 @@ private String unversionedFileName(String fileName) {
278272
}
279273
}
280274

281-
static String classFileWithoutSuffix(String result) {
275+
String classFileWithoutSuffix(String result) {
282276
return result.substring(0, result.length() - CLASS_EXTENSION.length());
283277
}
284278

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.Enumeration;
3939
import java.util.List;
4040
import java.util.Optional;
41+
import java.util.Set;
4142
import java.util.concurrent.ForkJoinPool;
4243
import java.util.concurrent.TimeUnit;
4344
import java.util.stream.Collectors;
@@ -418,10 +419,14 @@ public Class<?> loadClassFromModule(Object module, String className) throws Clas
418419
return classLoaderSupport.loadClassFromModule(module, className);
419420
}
420421

421-
public Optional<Object> findModule(String moduleName) {
422+
public Optional<? extends Object> findModule(String moduleName) {
422423
return classLoaderSupport.findModule(moduleName);
423424
}
424425

426+
public Set<String> findModuleNamesForPackage(String packageName) {
427+
return classLoaderSupport.findModuleNamesForPackage(packageName);
428+
}
429+
425430
public void processAddExportsAndAddOpens(OptionValues parsedHostedOptions) {
426431
classLoaderSupport.processAddExportsAndAddOpens(parsedHostedOptions);
427432
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Collections;
2929
import java.util.List;
3030
import java.util.Optional;
31+
import java.util.Set;
3132
import java.util.concurrent.ForkJoinPool;
3233

3334
import org.graalvm.compiler.options.OptionValues;
@@ -53,6 +54,11 @@ public Optional<Object> findModule(String moduleName) {
5354
return Optional.empty();
5455
}
5556

57+
@Override
58+
public Set<String> findModuleNamesForPackage(String packageName) {
59+
return Collections.emptySet();
60+
}
61+
5662
@Override
5763
void processAddExportsAndAddOpens(OptionValues parsedHostedOptions) {
5864
/* Nothing to do for Java 8 */
@@ -69,6 +75,6 @@ Class<?> loadClassFromModule(Object module, String className) throws ClassNotFou
6975

7076
@Override
7177
public void initAllClasses(ForkJoinPool executor, ImageClassLoader imageClassLoader) {
72-
new ClassInit(executor, imageClassLoader, this).init();
78+
new ClassInit(executor, imageClassLoader).init();
7379
}
7480
}

0 commit comments

Comments
 (0)