[GR-34108] Extend boot module layer to include required modules #3821

ivan-ristovic · 2021-09-20T10:42:16Z

This PR implements #3733

substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java

olpaw · 2021-09-30T08:08:20Z

substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java

+    /**
+     * This method creates Module instances that will populate the runtime boot module layer of the image.
+     * This implementation is copy-pasted from {@link java.lang.Module#defineModules(Configuration, Function, ModuleLayer)}
+     * with few simplifications (removing multiple classloader support) and removal of VM state updates


removal of VM state updates

😬 if java.lang.Module#defineModules would have been written in a way that partitions module creation from VM state update we could reuse the module creation part without this reflection-orgy.

olpaw

Looks good now.

Please also run mx eclipseformat --primary to see where you need to adjust the formatting.

olpaw · 2021-09-30T12:07:15Z

Unfortunately it e.g. fails mx gate --omit-clean -t 'native unittests'
...

[com.oracle.svm.junit.svmjunitrunner:5896]    classlist:   1,336.36 ms,  0.96 GB
[com.oracle.svm.junit.svmjunitrunner:5896]        (cap):     744.56 ms,  0.96 GB
[com.oracle.svm.junit.svmjunitrunner:5896]        setup:   4,608.89 ms,  0.96 GB
[com.oracle.svm.junit.svmjunitrunner:5896]     (clinit):     464.64 ms,  3.22 GB
[com.oracle.svm.junit.svmjunitrunner:5896]     analysis:  23,231.12 ms,  3.22 GB
Fatal error: com.oracle.svm.core.util.VMError$HostedError: Failed to synthesize the runtime boot module layer configuration.
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.util.VMError.shouldNotReachHere(VMError.java:76)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.ModuleLayerFeature.synthesizeRuntimeBootLayerConfiguration(ModuleLayerFeature.java:170)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.ModuleLayerFeature.synthesizeRuntimeBootLayer(ModuleLayerFeature.java:153)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.ModuleLayerFeature.afterAnalysis(ModuleLayerFeature.java:140)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.lambda$runPointsToAnalysis$13(NativeImageGenerator.java:752)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.FeatureHandler.forEachFeature(FeatureHandler.java:73)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:752)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:532)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:491)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:415)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:573)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:126)
Caused by: java.lang.module.FindException: Module junit not found
	at java.base/java.lang.module.Resolver.findFail(Resolver.java:877)
	at java.base/java.lang.module.Resolver.resolve(Resolver.java:128)
	at java.base/java.lang.module.Configuration.resolve(Configuration.java:422)
	at java.base/java.lang.module.Configuration.resolve(Configuration.java:256)
[com.oracle.svm.junit.svmjunitrunner:5896]      [total]:  29,619.83 ms,  3.22 GB
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.ModuleLayerFeature.synthesizeRuntimeBootLayerConfiguration(ModuleLayerFeature.java:168)
	... 10 more

The current solution is not able to deal with optional required modules (junit in this case). See

graal/substratevm/mx.substratevm/suite.py

Lines 1237 to 1241 in f49b9d1

    
           "requires" : [ 
        
               "static com.oracle.mxtool.junit", 
        
               "static junit", 
        
               "static hamcrest", 
        
           ],

Those are optional dependencies. I.e. we have to be able to handle the situation where these modules are not necessarily available during image build time.

olpaw · 2021-10-08T08:56:10Z

@ivan-ristovic please rebase to origin/master so that we get rid of the merge conflict.

substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java

olpaw

Running in CI now

olpaw · 2021-10-08T10:49:29Z

@ivan-ristovic I see style errors:

1. ERROR in /b/b/e/graal/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java (at line 165)
	private Configuration synthesizeRuntimeBootLayerConfiguration(List<Path> mp, Set<String> reachableModules) {
	                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The method synthesizeRuntimeBootLayerConfiguration(List<Path>, Set<String>) from the type ModuleLayerFeature can be declared as static
----------
2. ERROR in /b/b/e/graal/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java (at line 242)
	* This implementation is copy-pasted from {@link java.lang.Module#defineModules(Configuration, Function, ModuleLayer)}
	                                                                  ^^^^^^^^^^^^^
Javadoc: The method defineModules(Configuration, Function<String,ClassLoader>, ModuleLayer) from the type Module is not visible

olpaw · 2021-10-08T10:51:21Z

And JDK 17 gate errors like

[libjvmcicompiler:5415]    classlist:     994.78 ms,  0.96 GB
[libjvmcicompiler:5415]        setup:     369.36 ms,  0.96 GB
Fatal error: com.oracle.svm.util.ReflectionUtil$ReflectionUtilError: java.lang.NoSuchFieldException: EVERYONE_MODULE
	at com.oracle.svm.util.ReflectionUtil.lookupField(ReflectionUtil.java:114)
	at com.oracle.svm.util.ReflectionUtil.lookupField(ReflectionUtil.java:101)
	at com.oracle.svm.util.ReflectionUtil.readField(ReflectionUtil.java:121)
	at com.oracle.svm.hosted.ModuleLayerFeature$NameToModuleSynthesizer.<init>(ModuleLayerFeature.java:230)
	at com.oracle.svm.hosted.ModuleLayerFeature.afterRegistration(ModuleLayerFeature.java:110)
	at com.oracle.svm.hosted.NativeImageGenerator.lambda$setupNativeImage$15(NativeImageGenerator.java:828)
	at com.oracle.svm.hosted.FeatureHandler.forEachFeature(FeatureHandler.java:73)
	at com.oracle.svm.hosted.NativeImageGenerator.setupNativeImage(NativeImageGenerator.java:828)
	at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:530)
	at com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:491)
[libjvmcicompiler:5415]      [total]:   1,388.03 ms,  0.96 GB
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:415)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:573)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:126)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner$JDK9Plus.main(NativeImageGeneratorRunner.java:603)
Caused by: java.lang.NoSuchFieldException: EVERYONE_MODULE
	at java.base/java.lang.Class.getDeclaredField(Class.java:2610)
	at com.oracle.svm.util.ReflectionUtil.lookupField(ReflectionUtil.java:106)
	... 13 more

ivan-ristovic · 2021-10-09T13:25:46Z

Style issues fixed.

I believe the reflection error that you sent is something we have seen earlier and is related to it behaving differently on 16+. If you recall we discussed a similar "reflection bug" where the field is present in a class and reflection for some reason does not pick it up. Is this the same issue then, because as far as I see there is no reason why EVERYONE_MODULE cannot be accessed via reflection (it is present in the Module class in the same way as on 11)?

ivan-ristovic · 2021-10-15T17:14:06Z

I have tracked the problem down to filters in jdk.internal.reflect.Reflection. Feature is updated and tests pass locally, I have also added a comment on ReflectionUtil so that in the future people don't waste time debugging.

olpaw · 2021-10-21T13:19:43Z

@ivan-ristovic CI passes now mostly. Except for one little style issue:

1. ERROR in /b/b/e/main/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java (at line 258)
	private Field findFieldByName(Field[] fields, String name) {
	              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The method findFieldByName(Field[], String) from the type ModuleLayerFeature.NameToModuleSynthesizer can be declared as static
----------
1 problem (1 error)

olpaw · 2021-10-21T16:02:28Z

@ivan-ristovic ... aaand one more style issue:

gate: 21 Oct 2021 15:29:57(+02:53) BEGIN: CodeFormatCheck
Running: mx --kill-with-sigquit --strict-compliance eclipseformat -e /sc/bb/downloads/eclipse-4.14.0-linux-amd64_8aabce73d3/eclipse/eclipse --primary
we have: 5 batches
Processing batch 1 (1836 files)...
Processing batch 2 (98 files)...
Processing batch 3 (16 files)...
Processing batch 4 (2 files)...
Processing batch 5 (7 files)...
1 files were modified
 - src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
Changes:
--- a/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
+++ b/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
@@ -117,9 +117,9 @@
     public void beforeAnalysis(BeforeAnalysisAccess access) {
         FeatureImpl.BeforeAnalysisAccessImpl accessImpl = (FeatureImpl.BeforeAnalysisAccessImpl) access;
         Set<String> baseModules = ModuleLayer.boot().modules()
-                .stream()
-                .map(Module::getName)
-                .collect(Collectors.toSet());
+                        .stream()
+                        .map(Module::getName)
+                        .collect(Collectors.toSet());
         ModuleLayer runtimeBootLayer = synthesizeRuntimeBootLayer(accessImpl.imageClassLoader, baseModules);
         BootModuleLayerSupport.instance().setBootLayer(runtimeBootLayer);
     }
@@ -130,16 +130,16 @@
         AnalysisUniverse universe = accessImpl.getUniverse();
 
         Stream<Module> analysisReachableModules = universe.getTypes()
-                .stream()
-                .filter(t -> t.isReachable() && !t.isArray())
-                .map(t -> t.getJavaClass().getModule())
-                .distinct();
+                        .stream()
+                        .filter(t -> t.isReachable() && !t.isArray())
+                        .map(t -> t.getJavaClass().getModule())
+                        .distinct();
 
         Set<String> allReachableModules = analysisReachableModules
-                .filter(Module::isNamed)
-                .filter(m -> !m.getDescriptor().modifiers().contains(ModuleDescriptor.Modifier.SYNTHETIC))
-                .flatMap(ModuleLayerFeature::extractRequiredModuleNames)
-                .collect(Collectors.toSet());
+                        .filter(Module::isNamed)
+                        .filter(m -> !m.getDescriptor().modifiers().contains(ModuleDescriptor.Modifier.SYNTHETIC))
+                        .flatMap(ModuleLayerFeature::extractRequiredModuleNames)
+                        .collect(Collectors.toSet());
 
         ModuleLayer runtimeBootLayer = synthesizeRuntimeBootLayer(accessImpl.imageClassLoader, allReachableModules);
         BootModuleLayerSupport.instance().setBootLayer(runtimeBootLayer);
@@ -203,19 +203,19 @@
         @Override
         public Optional<ModuleReference> find(String name) {
             return ModuleLayer.boot()
-                    .configuration()
-                    .findModule(name)
-                    .map(ResolvedModule::reference);
+                            .configuration()
+                            .findModule(name)
+                            .map(ResolvedModule::reference);
         }
 
         @Override
         public Set<ModuleReference> findAll() {
             return ModuleLayer.boot()
-                    .configuration()
-                    .modules()
-                    .stream()
-                    .map(ResolvedModule::reference)
-                    .collect(Collectors.toSet());
+                            .configuration()
+                            .modules()
+                            .stream()
+                            .map(ResolvedModule::reference)
+                            .collect(Collectors.toSet());
         }
     }
 
@@ -260,21 +260,22 @@
         }
 
         /**
-         * This method creates Module instances that will populate the runtime boot module layer of the image.
-         * This implementation is copy-pasted from Module#defineModules(Configuration, Function, ModuleLayer)
-         * with few simplifications (removing multiple classloader support) and removal of VM state updates
-         * (otherwise we would be re-defining modules to the host VM).
+         * This method creates Module instances that will populate the runtime boot module layer of
+         * the image. This implementation is copy-pasted from Module#defineModules(Configuration,
+         * Function, ModuleLayer) with few simplifications (removing multiple classloader support)
+         * and removal of VM state updates (otherwise we would be re-defining modules to the host
+         * VM).
          */
         Map<String, Module> synthesizeNameToModule(ModuleLayer runtimeBootLayer, ClassLoader cl)
-                throws IllegalAccessException, InvocationTargetException, InstantiationException {
+                        throws IllegalAccessException, InvocationTargetException, InstantiationException {
             Configuration cf = runtimeBootLayer.configuration();
 
             int cap = (int) (cf.modules().size() / 0.75f + 1.0f);
             Map<String, Module> nameToModule = new HashMap<>(cap);
 
             /*
-             * Remove mapping of modules to classloaders.
-             * Create module instances without defining them to the VM
+             * Remove mapping of modules to classloaders. Create module instances without defining
+             * them to the VM
              */
             for (ResolvedModule resolvedModule : cf.modules()) {
                 ModuleReference mref = resolvedModule.reference();
@@ -286,8 +287,8 @@
             }
 
             /*
-             * Setup readability and exports/opens.
-             * This part is unchanged, save for field setters and VM update removals
+             * Setup readability and exports/opens. This part is unchanged, save for field setters
+             * and VM update removals
              */
             for (ResolvedModule resolvedModule : cf.modules()) {
                 ModuleReference mref = resolvedModule.reference();

…al class

graalvmbot added the oca-signed label Sep 20, 2021

oubidar-Abderrahim self-assigned this Sep 22, 2021

oubidar-Abderrahim requested review from christianwimmer, vjovanov and olpaw September 22, 2021 13:40

olpaw requested changes Sep 30, 2021

View reviewed changes

olpaw added this to the 22.0 milestone Sep 30, 2021

olpaw requested changes Sep 30, 2021

View reviewed changes

olpaw changed the title ~~Extend boot module layer to include required modules~~ [GR-34108] Extend boot module layer to include required modules Sep 30, 2021

olpaw linked an issue Sep 30, 2021 that may be closed by this pull request

Issues related to boot module layer synthesizing with closed-world approach #3733

Closed

olpaw reviewed Oct 8, 2021

View reviewed changes

substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java Outdated Show resolved Hide resolved

olpaw approved these changes Oct 8, 2021

View reviewed changes

olpaw mentioned this pull request Oct 21, 2021

Module support for native-image building #1962

Closed

ivan-ristovic added 7 commits October 22, 2021 11:02

Extend boot module layer to include required modules

165b245

Move reflection fields and nameToModule synthesizing logic into a fin…

0b1dc17

…al class

Support for static required modules

93c9ed9

Rename notFoundModules to missingModules

6d81689

Style fixes

6255d13

Add workaround for reflection filters

d39874f

Fix several style issues

8d88db0

graalvmbot merged commit f8c2041 into oracle:master Oct 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[GR-34108] Extend boot module layer to include required modules #3821

[GR-34108] Extend boot module layer to include required modules #3821

Uh oh!

ivan-ristovic commented Sep 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olpaw Sep 30, 2021

Uh oh!

olpaw left a comment

Uh oh!

olpaw commented Sep 30, 2021 •

edited

Loading

Uh oh!

olpaw commented Oct 8, 2021

Uh oh!

Uh oh!

olpaw left a comment

Uh oh!

olpaw commented Oct 8, 2021

Uh oh!

olpaw commented Oct 8, 2021

Uh oh!

ivan-ristovic commented Oct 9, 2021

Uh oh!

ivan-ristovic commented Oct 15, 2021

Uh oh!

olpaw commented Oct 21, 2021

Uh oh!

olpaw commented Oct 21, 2021 •

edited

Loading

Uh oh!

Uh oh!

[GR-34108] Extend boot module layer to include required modules #3821

[GR-34108] Extend boot module layer to include required modules #3821

Uh oh!

Conversation

ivan-ristovic commented Sep 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olpaw Sep 30, 2021

Choose a reason for hiding this comment

Uh oh!

olpaw left a comment

Choose a reason for hiding this comment

Uh oh!

olpaw commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olpaw commented Oct 8, 2021

Uh oh!

Uh oh!

olpaw left a comment

Choose a reason for hiding this comment

Uh oh!

olpaw commented Oct 8, 2021

Uh oh!

olpaw commented Oct 8, 2021

Uh oh!

ivan-ristovic commented Oct 9, 2021

Uh oh!

ivan-ristovic commented Oct 15, 2021

Uh oh!

olpaw commented Oct 21, 2021

Uh oh!

olpaw commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

olpaw commented Sep 30, 2021 •

edited

Loading

olpaw commented Oct 21, 2021 •

edited

Loading