RFR (L) 8194812: Extend class-data sharing to support the module path

[Ioi Lam]

Hi Calvin,

Some comments on the tests:


[1] For all the new tests, I think we should avoid using 
TestCommon.execCommon.
execCommon is error prone. See 
https://bugs.openjdk.java.net/browse/JDK-8179249

For example, in MainModuleOnly.java

+        output = TestCommon.execCommon( "-Xlog:class+load=trace",
+                                        "-cp", destJar.toString(),
+                                        "--module-path", 
moduleDir.toString(),
+                                        "-m", TEST_MODULE1);
+        TestCommon.checkExecReturn(output, 0, true,
+              "[class,load] com.simple.Main source: shared objects file");

In the past we often have direct checks on <output> which would fail
due to mapping failure.

It's must safer (and more readable) to change the code to:

TestCommon.run( "-Xlog:class+load=trace",
     "-cp", destJar.toString(),
     "--module-path", moduleDir.toString(),
     "-m", TEST_MODULE1)
   .assertNormalExit("[class,load] com.simple.Main source: shared 
objects file");


You can read comments near the CDSTestUtils.Result class for more 
information on
the different forms of assertXXX().

Also, TestCommon.execModule has the same problems as execCommon. For 
example,
in ModulePathAndCP.java

+        output = TestCommon.execModule(prefix,
+                                       null, // --upgrade-module-path
+                                       moduleDir2.toString(), // 
--module-path
+                                       MAIN_MODULE); // -m
+        output.shouldHaveExitValue(0)
+              .shouldMatch(".class.load. com.greetings.Main 
source:.*com.greetings.jar")
+              .shouldMatch(".class.load. org.astro.World 
source:.*org.astro.jar");

This code forgets to call checkExecReturn.

I think it's better to use a new method TestCommon.runWithModule, like this:

       http://cr.openjdk.java.net/~iklam/jdk11/runWithModule/

Then the above code can be changed to

     TestCommon.runWithModule(....)
       .assertNormalExit(out -> {
         out.shouldMatch(".class.load. com.greetings.Main 
source:.*com.greetings.jar")
            .shouldMatch(".class.load. org.astro.World 
source:.*org.astro.jar");
       });


[2] in BootAppendTests.java

       output.shouldHaveExitValue(0);
       if (!TestCommon.isUnableToMap(output))

Here, the isUnableToMap check is not necessarily because we
know for sure mapping will not happen.

I think we should add a new CDSUtils.assertCDSSilentlyDisabled() for
for all the tests for --limit-modules, --patch-module and
--upgrade-module-path.


   TestCommon.run(
       appJar,
       "-Xbootclasspath/a:" + bootAppendJar,
       "--limit-modules", "java.base",
       "-XX:+TraceClassLoading",
       MAIN_CLASS,
       "Test #4", BOOT_APPEND_MODULE_CLASS, "true", "BOOT")
     .assertCDSSilentlyDisabled(output -> {
             output.shouldHaveExitValue(0);
                   .shouldMatch(".class.load. sun.nio.cs.ext.MyClass 
source:.*bootAppend.jar");
         });


   // When {--limit-modules, --patch-module, and/or --upgrade-module-path}
   // are specified, CDS is silently disabled for both -Xshare:auto and 
-Xshare:on.
   public Result assertSilentlyDisabledCDS(Checker checker) throws 
Exception {
       if (hasMappingFailure) {
           throw new RuntimeException("Unexpected mapping failure");
       }
       // this comes from a JVM warning message.
       output.shouldContain("CDS is disabled");

       checker.check(output);
       return this;
   }


[3] Some tests have this pattern:

    OutputAnalyzer output = TestCommon.createArchive(
                                    null, appClasses,
                                    "--module-path", moduleDir.toString(),
                                    "--add-modules",
                                    MAIN_MODULE1 + "," + MAIN_MODULE2);
    TestCommon.checkExecReturn(output, 0, true, "Loading classes to share");

Here you shouldn't use TestCommon.checkExecReturn, because mapping failure
will never happen during -Xshare:dump.

Thanks
- Ioi


On 4/2/18 2:30 PM, Calvin Cheung wrote:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8194812
>
> webrev: http://cr.openjdk.java.net/~ccheung/8194812/webrev.00/
>
> Design write-up: 
> http://cr.openjdk.java.net/~ccheung/8194812/write-up/module-path-design.pdf
>
> Testing: hs-tier1 through hs-tier4 (in progress, no new failures so far).
>
> thanks,
> Calvin