RFR[S] 8191927 Enable AppCDS for custom loaders on all 64-bit Linux and AIX

[Ioi Lam]

Hi Volker,

Thanks for the review


On 11/28/17 8:46 AM, Volker Simonis wrote:
> Hi Ioi,
>
> the change and your refactorings to it look good :) I specially like
> the introduction of  "@requires vm.cds.custom.loaders" to simplify the
> logic.
>
> I've only found two little problems:
>
> - the patch for the test
> "cacheObject/CheckCachedResolvedReferences.java" is empty. That's
> probably because it is not related to AppCDS. But the test can and
> should still run on all 64-bit Linux and AIX platfroms as well.

I've fixed this. I thought it's unrelated to custom loaders, but it actually
does. I've added a comment to reflect that:


--- a/CheckCachedResolvedReferences.java    Tue Nov 28 21:04:42 2017 +0530
+++ b/CheckCachedResolvedReferences.java    Tue Nov 28 09:33:46 2017 -0800
@@ -26,8 +26,7 @@
   * @test
   * @summary Test resolved_references
   * @requires (vm.opt.UseCompressedOops == null) | 
(vm.opt.UseCompressedOops == true)
- * @requires (sun.arch.data.model == "64")
- * @requires ((os.family == "linux") & (os.arch=="amd64")) | (os.family 
== "solaris")
+ * @requires vm.cds.custom.loaders
   * @requires (vm.gc=="null")
   * @library /test/lib /test/hotspot/jtreg/runtime/appcds
   * @modules java.base/jdk.internal.misc
@@ -53,9 +52,9 @@
          String helloJarPath = ClassFileInstaller.getJarPath("hello.jar");

          String classlist[] = new String[] {
-            "CheckCachedResolvedReferencesApp",
-            "java/lang/Object id: 1",
-            "Hello id: 2 super: 1 source: " + helloJarPath
+            "CheckCachedResolvedReferencesApp",            // built-in 
app loader
+            "java/lang/Object id: 1",                      // boot loader
+            "Hello id: 2 super: 1 source: " + helloJarPath // custom loader
          };

          TestCommon.testDump(appJar, classlist, use_whitebox_jar);



> - the following two tests still get executed in the product build
> where they fail because they use non-product flags:
> runtime/appcds/DumpClassList.java
> runtime/appcds/ProhibitedPackage.java
>
> I think you fixed that in one of your previous patches but it probably
> got lost again before pushing.

I wanted to fix them in a separate changeset, so that the "move appcds 
to open"
changeset is a more verbatim move. It's the following bug, and I will 
post RFR shortly.

https://bugs.openjdk.java.net/browse/JDK-8191747
"[TESTBUG] runtime/AppCDS/DumpClassList.java and ProhibitedPackage.java 
fail on product bits"

Instead of "@require vm.debug", I'll try to rewrite the test to remove the
dependency on -XX:+PrintSystemDictionaryAtExit

Thanks
- Ioi

> The attached "addon.patch" fixes these two problems.
>
> Thank you and bet regards,
> Volker
>
>
> On Mon, Nov 27, 2017 at 10:42 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>> Hi,
>>
>> Please review this change to support AppCDS on all 64-bit Linux platforms as
>> well as AIX.
>>
>>      https://bugs.openjdk.java.net/browse/JDK-8191927
>> http://cr.openjdk.java.net/~iklam/jdk10/8191927-appcds-custom-loader-for-more-platforms.v01/
>>
>> The patch must be applied on top of the AppCDS patch:
>>
>> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds.v03/
>>
>> The patch was initially contributed by Volker Simonis. I modified it a
>> little to avoid repeating the platform enumerations in all the test cases.
>> Instead, I added a new test property, so the tests that require AppCDS
>> support for custom loaders can be written as:
>>
>>      * @requires vm.cds.custom.loaders
>>
>> I've tested on Linux manually, and I am now running on all of the
>> Oracle-supported platforms using our internal test harness.
>>
>> Thanks
>> - Ioi