[GR-30957] Implement Module/ModuleLayer substitutions #3445
Conversation
|
@ivan-ristovic please provide a before-after comparison based on a simple hello-world module-build sample so that it is clear how the runtime-representation of module info is changed by this PR. |
.../src/com.oracle.svm.core.jdk11/src/com/oracle/svm/core/jdk/Target_java_lang_WeakPairMap.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
Outdated
Show resolved
Hide resolved
I added a simple hello-module sample output to the PR description |
substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
Outdated
Show resolved
Hide resolved
|
|
Running your PR on top of #3446 breaks When setting module runtime-state you must honor |
Update. Forget what I just said. Your changes actually fixe the hellomodule test now correctly requiring I.e. you should apply the following patch to your branch: diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py
index 0ecb3c4ed7f..ed577488c50 100644
--- a/substratevm/mx.substratevm/mx_substratevm.py
+++ b/substratevm/mx.substratevm/mx_substratevm.py
@@ -1095,7 +1095,7 @@ def hellomodule(args):
built_image = native_image([
'--verbose', '-ea', '-H:Path=' + build_dir,
'--add-exports=moduletests.hello.lib/hello.privateLib=moduletests.hello.app',
- '--add-exports=moduletests.hello.lib/hello.privateLib2=moduletests.hello.app',
+ '--add-opens=moduletests.hello.lib/hello.privateLib2=moduletests.hello.app',
'-p', module_path_sep.join(module_path), '-m', 'moduletests.hello.app'])
mx.log('Running image ' + built_image + ' built from module:')
mx.run([built_image])
diff --git a/substratevm/src/native-image-module-tests/hello.app/src/main/java/hello/Main.java b/substratevm/src/native-image-module-tests/hello.app/src/main/java/hello/Main.java
index be8b8025354..5c1a61a8967 100644
--- a/substratevm/src/native-image-module-tests/hello.app/src/main/java/hello/Main.java
+++ b/substratevm/src/native-image-module-tests/hello.app/src/main/java/hello/Main.java
@@ -40,7 +40,7 @@ public class Main {
assert helloLibModule.isExported("hello.lib");
assert helloAppModule.canRead(helloLibModule);
- // assert !helloLibModule.canRead(helloAppModule); GR-30957
+ assert !helloLibModule.canRead(helloAppModule);
System.out.println("Basic Module test involving " + helloAppModule + " and " + helloLibModule);
Greeter.greet(); |
...atevm/src/com.oracle.svm.core.jdk11/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java
Outdated
Show resolved
Hide resolved
Done. |
|
Hi @ivan-ristovic |
Done. |
...ratevm/src/com.oracle.svm.core.jdk11/src/com/oracle/svm/core/jdk/BootModuleLayerSupport.java
Outdated
Show resolved
Hide resolved
...ratevm/src/com.oracle.svm.core.jdk11/src/com/oracle/svm/core/jdk/BootModuleLayerSupport.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/BootModuleLayerSupport.java
Outdated
Show resolved
Hide resolved
olpaw
changed the title
substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
Outdated
Show resolved
Hide resolved
|
@olpaw About the module/package bookkeeping/lookup: at this point, it is nececary to check if the module with a given name has already been defined: I was hoping that also the package lookup could also be more efficient as well (if the loader has already defined a package with a given name). |
You can iterate the modules in the bootLayer at runtime and access for each Module |
...atevm/src/com.oracle.svm.core.jdk11/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java
Outdated
Show resolved
Hide resolved
@ivan-ristovic your PR breaks Java 8 compatibility. If you use JAVA_HOME with Java 8 and EXTRA_JAVA_HOMES with Java 11 you will see errors like: Maybe a side effect of removing the Also |
Unfortunately, I do not have EE access yet. CE built locally with
I agree, however that will cause problems as several other classes use these types (that is why I kept the overlays) for example: I am not sure what is the best approach here, maybe you have some ideas? On a side note, PR is rebased and commits are squashed. I have also added checks for already defined modules/packages. |
That just means |
@ivan-ristovic the issue described in #3445 (comment) can also easily be reproduced on your side with.
results in |
|
Locally this now passes, hope there are no issues on EE as well. |
|
@ivan-ristovic , rerunning in our CI now ... |
|
@ivan-ristovic CI results look better now but I see failing JDK16 gates. E.g. You should be able to reproduce that easily by setting JAVA_HOME to a JDK-16 and trying to build e.g. libpolyglot (use |
|
@ivan-ristovic, to get rid of the ===================================================================
diff --git a/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java b/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java
--- a/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java (revision Staged)
+++ b/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java (date 1629218976138)
@@ -95,7 +95,7 @@
@Override
public boolean isInConfiguration(IsInConfigurationAccess access) {
- return new JDK11OrLater().getAsBoolean();
+ return new JDK11OrLater().getAsBoolean() && ModuleLayer.boot().findModule("org.graalvm.nativeimage.builder").isPresent();
}
@Overrideso that the code in |
|
Excellent idea, added! |
|
After @ivan-ristovic and @gradinac found another case of a missing
|
Add hellomodule gate Restrict access to Module$ReflectionData Remove unnececary overlay for BootModuleLayerSupport Synthesize boot layer in a safer way
Substitute native methods of the Module class Fix boot layer configuration resolving Expand `mx hellomodule` with more boot layer tests
Implement checks inside Module native methods Calculate boot layer once after analysis Implement module/package checks in runtime for `defineModule0`
Update overlays related to module system Specific `defineModule0` implementation for JDK16 Add dependency to `com.oracle.svm.core.jdk11` from `com.oracle.svm.core.jdk15`
|
... running in internal CI now. @ivan-ristovic @gradinac |
This PR implements basic methods of the
Moduleclass and synthesizes the runtime boot module layer.Change list:
canRead(),canUse()etc. because the JDK implementation can be reused@Deleteannotation fromReflectionData, because it is used from now onReflectionData.usesfield, otherwise it will contain classes that were not nececarily discovered by analysisjava.lang.WeakHashMap, in order to redefine theReflectionData.usesfieldModuleLayer.boot()method to return the synthesized layermx hellomodulemx hellomoduleModuleclassBoot layer module set comparison (for a simple modular application similar to
mx hellomodule)Hotspot:
SVM (before this PR):
SVM (with changes introduced by this PR):
Note that it is trivial to filter the set in order to hide modules we do not wish to expose, currently all non-synthesized reachable modules are included. A module is reachable when a class within it is reachable.