RFR(M) 8192921 Improve CDS support for custom loaders

[yumin qi]

Hi, Ioi

  Thanks for fixing the custom loader issue in CDS. It looks good!
Only a few minor suggestions.

1) classListParser.cpp:

     } else if (parse_int_option("super:", &_super)) {-
check_already_loaded("Super class", _super);-      continue;+
     }

According to current design, DumpLoadedClassList will output
java/lang/Object id : 1
....
So except for Object, every other class will have "super : <id>"
There is no checking for return false. If format damages leads to fail to
parse the value, should we do something here?

2) systemDictionaryShared.cpp:

bool SystemDictionaryShared::check_hierarchy(...):

Klass* k = resolve_super_or_fail(ik->name(), expected_intf->name(),
class_loader, protection_domain, false, CHECK_NULL);

Since the function return bool, I think here should be CHECK_false,
that is, with Exception, it returns false.

3) TestCommon.java:

+    static byte[] asciibytes(String s) {+        byte b[] = new
byte[s.length()];+        for (int i=0; i<b.length; i++) {+
b[i] = (byte)s.charAt(i);+        }+        return b;+    }

Maybe just use String.getBytes:

return s.getBytes(StandardCharsets.US_ASCII);

4) For many loops (C/C++ part), space should be added before and after
"=", "<". I don't know if it is still recommended coding in that
style.


Thanks

Yumin







On Fri, Dec 21, 2018 at 9:19 PM Ioi Lam <ioi.lam at oracle.com> wrote:

>
> http://cr.openjdk.java.net/~iklam/jdk13/8192921-cds-cust-loader-improve.v01/
> https://bugs.openjdk.java.net/browse/JDK-8192921
>
>
> ISSUE:
>
> In JDK 12, there's experimental support for custom loader in CDS.
> However, there are limitations:
>
> 1. if two or more custom loaders want to load a class named "A", only
>     one of those loaders can actually succeed in loading an archived
>     class.
>
> 2. the classlist needs to be hand-crafted (or by a script that parses
>     the output of -Xlog:class+load=debug).
>
> FIX:
>
> 1. Now you can archive multiple classes of the same name. At run
>     time, we select an archived class that
>     (a) matches the fingerprint of the class file, and
>     (b) matches the class hierarchy of the current loader.
>
>     (b) is the tricky part. See
> SystemDictionaryShared::has_same_hierarchy_for_unregistered_class
>
>     For a test case, see DuplicatedClassesTest.java
>
> 2. -XX:DumpLoadedClassList now automatically includes classes loaded
>     by custom loaders. The generated class list includes all the
>     necessary attributes for custom loader support, such as "super:",
>     "source:" and "interface:".
>
> I have tested with Eclipse IDE, which loads most of its classes with
> custom loaders. I am planning to add more test cases.
>
> Thanks
> - Ioi
>
>
>