[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: RFR (XS) 8185160 -XX:DumpLoadedClassList omits graal classes
From: Jiangli Zhou <jiangli.zhou () oracle ! com>
Date: 2017-10-20 20:13:38
Message-ID: AAC0240C-9ACE-4177-9F33-107AE6A50F36 () oracle ! com
[Download RAW message or body]
> On Oct 20, 2017, at 1:05 PM, Ioi Lam <ioi.lam@oracle.com> wrote:
>
>
>
> On 10/19/17 5:02 PM, Jiangli Zhou wrote:
> >
> > > On Oct 19, 2017, at 4:06 PM, Ioi Lam <ioi.lam@oracle.com \
> > > <mailto:ioi.lam@oracle.com>> wrote:
> > >
> > >
> > > On 10/19/17 2:57 PM, Jiangli Zhou wrote:
> > > > Hi Ioi,
> > > >
> > > > > On Oct 19, 2017, at 10:55 AM, Ioi Lam <ioi.lam@oracle.com \
> > > > > <mailto:ioi.lam@oracle.com>> wrote:
> > > > >
> > > > >
> > > > > On 10/19/17 9:47 AM, Jiangli Zhou wrote:
> > > > > >
> > > > > > > On Oct 18, 2017, at 9:50 PM, Ioi Lam <ioi.lam@oracle.com \
> > > > > > > <mailto:ioi.lam@oracle.com>> wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 10/18/17 9:35 PM, Jiangli Zhou wrote:
> > > > > > > > Hi Ioi,
> > > > > > > >
> > > > > > > > > On Oct 18, 2017, at 4:54 PM, Ioi Lam <ioi.lam@oracle.com \
> > > > > > > > > <mailto:ioi.lam@oracle.com>> wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On 10/17/17 9:04 PM, David Holmes wrote:
> > > > > > > > > > Hi Ioi,
> > > > > > > > > >
> > > > > > > > > > On 18/10/2017 1:52 PM, Ioi Lam wrote:
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8185160 \
> > > > > > > > > > > <https://bugs.openjdk.java.net/browse/JDK-8185160>
> > > > > > > > > > > Summary:
> > > > > > > > > > >
> > > > > > > > > > > DumpLoadedClassList skips classes that can't be stored into the \
> > > > > > > > > > > CDS archive. For boot and platform loaders, only classes from \
> > > > > > > > > > > the JRT image are listed.
> > > > > > > > > > >
> > > > > > > > > > > The test for "is this class from JRT image" was insufficient. \
> > > > > > > > > > > It checked only classes whose source ends with "...../modules". \
> > > > > > > > > > > For the platform loader, which loads the graal classes, the \
> > > > > > > > > > > source is in the form "jrt:/jdk.internal.vm.compiler".
> > > > > > > > > > >
> > > > > > > > > > > The fix is to also check for a "jrt:" prefix.
> > > > > > > > > >
> > > > > > > > > > So ./share/classfile/classLoader.hpp:
> > > > > > > > > >
> > > > > > > > > > static bool is_jrt(const char* name) { return \
> > > > > > > > > > string_ends_with(name, MODULES_IMAGE_NAME); }
> > > > > > > > > > is a badly named function or just plain broken?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I've renamed all the is_jrt() functions to is_modules_image(), so \
> > > > > > > > > it's clear as to what it does.
> > > > > > > > > See \
> > > > > > > > > http://cr.openjdk.java.net/~iklam/jdk10/8185160-dump-class-list-omits-graal.v02/ \
> > > > > > > > > <http://cr.openjdk.java.net/%7Eiklam/jdk10/8185160-dump-class-list-omits-graal.v02/>
> > > > > > > > >
> > > > > > > >
> > > > > > > > The logic looks puzzling. For both NULL class loader and the \
> > > > > > > > PlatformClassLoader:
> > > > > > > > The first if check at line 5938 excludes any classes not in the \
> > > > > > > > jimage.
> > > > > > > > 5938 if (!ClassLoader::is_modules_image(stream->source()) \
> > > > > > > > && strncmp(stream->source(), "jrt:", 4) != 0) { 5939 skip \
> > > > > > > > = true; 5940 }
> > > > > > > > The second if check at line 5942 skips any classes from the boot \
> > > > > > > > append class path loaded by the NULL class loader. The second check \
> > > > > > > > skips a subset of the classes that's already skipped by the first \
> > > > > > > > check. The second check is redundant.
> > > > > > > You're right. The second check is redundant. I'll remove it.
> > > > > > >
> > > > > > > > 5942 if (class_loader == NULL && \
> > > > > > > > ClassLoader::contains_append_entry(stream->source())) { 5943 \
> > > > > > > > // For the boot loader, also skip all classes that are loaded from \
> > > > > > > > the appended entries 5944 skip = true;
> > > > > > > > 5945 }
> > > > > > > > What about the classes from -Xbootclasspath/a (not in --patch-module \
> > > > > > > > and --upgrade-module-path)?
> > > > > > > I am not sure what you mean here.
> > > > > >
> > > > > > This excludes all classes from the -Xbootclasspath/a. However, classes \
> > > > > > from -Xbootclasspath/a can be archived and used at runtime properly.
> > > > >
> > > > > Hi Jiangli,
> > > > >
> > > > > You're right. I misunderstood the original comments about the treatment of \
> > > > > the bootclasspath/a classes. Here's the updated code that should be easier \
> > > > > to understand:
> > > > > bool skip = false;
> > > > > if (class_loader == NULL || \
> > > > > SystemDictionary::is_platform_class_loader(class_loader)) { // For the boot \
> > > > > and platform class loaders, skip classes that are not found in the // java \
> > > > > runtime image, such as those found in the --patch-module entries. // These \
> > > > > classes can't be loaded from the archive during runtime. if \
> > > > > (!ClassLoader::is_modules_image(stream->source()) && \
> > > > > strncmp(stream->source(), "jrt:", 4) != 0) { skip = true;
> > > > > }
> > > > >
> > > > > if (class_loader == NULL && \
> > > > > ClassLoader::contains_append_entry(stream->source())) { // .. but don't \
> > > > > skip the boot classes that are loaded from -Xbootclasspath/a // as they can \
> > > > > be loaded from the archive during runtime. skip = false;
> > > > > }
> > > > > }
> > > >
> > > > Can you please check with a exploded build if the above also works? What is \
> > > > the stream->source() from classes from exploded build loaded by the NULL \
> > > > loader and the PlatformClassLoader?
> > >
> > > Hi Jiangli,
> > >
> > > I checked the exploded build and all classes are skipped now, because they are \
> > > not loaded from the $JAVA_HOME/lib/modules file. The stream->source looks like \
> > > this:
> > > skip writing class sun/security/util/FilePermCompat from source \
> > > /jdk/bld/cons/jdk/modules/java.base to classlist file
> > > However, I am not sure if this is relevant, since CDS is not supported with the \
> > > exploded build.
> > > BTW, DumpLoadedClassList is used during the JDK build process to generate the \
> > > classlist file. However, this uses the interim image, which is not an exploded \
> > > build:
> > > open/make/GenerateLinkOptData.gmk: $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java \
> > > -XX:DumpLoadedClassList=$@ \
> > >
> > > $ ./support/interim-image/bin/java -cp \
> > > /jdk/tmp/jtreg/work/classes/2/runtime/AppCDS/DumpClassList.d:/jdk/cons/closed/te \
> > > st/hotspot/jtreg/runtime/AppCDS:/jdk/tmp/jtreg/work/classes/2/open/test/lib:/jdk \
> > > /cons/open/test/lib:/java_re2/misc/promoted/jtreg/4.2/fcs/b09/binaries/jtreg/lib/javatest.jar:/jtreg/4.2/fcs/b09/binaries/jtreg/lib/jtreg.jar \
> > > -XX:DumpLoadedClassList=app.list \
> > > --patch-module=java.base=/jdk/tmp/jtreg/work/classes/2/runtime/AppCDS/DumpClassList.d/javabase.jar \
> > > -Xbootclasspath/a:/jdk/tmp/jtreg/work/classes/2/runtime/AppCDS/DumpClassList.d/bootappend.jar \
> > > -cp /jdk/tmp/jtreg/work/classes/2/runtime/AppCDS/DumpClassList.d/app.jar \
> > > ArrayListTest hello world.
> > > skip writing class java/lang/NewClass from source \
> > > /jdk/tmp/jtreg/work/classes/2/runtime/AppCDS/DumpClassList.d/javabase.jar to \
> > > classlist file NewClass
> > > class java.lang.NewClass
> > > Foo
> > > class boot.append.Foo
> > > ioilinux ~/jdk/bld/cons$ wc app.list
> > > 701 701 22630 app.list
> >
> >
> > That's what I expected. Thanks for verifying it. Since the goal here is to \
> > generate a more "precise" classlist for CDS and CDS does not support exploded \
> > build, it would make sense to print a warning and disable DumpLoadedClassList if \
> > ClassLoader::has_jrt_entry() is not true.
>
> Hi Jiangli,
>
> Thanks for the suggestion. I've added the warning as you suggested:
>
>
> if (DumpLoadedClassList != NULL && stream->source() != NULL && \
> classlist_file->is_open()) {
> ! if (!ClassLoader::has_jrt_entry()) {
> ! warning("DumpLoadedClassList and CDS are not supported in exploded build");
> ! DumpLoadedClassList = NULL;
> ! } else if (SystemDictionaryShared::is_sharing_possible(_loader_data) &&
> _host_klass == NULL) {
> // Only dump the classes that can be stored into CDS archive.
> // Anonymous classes such as generated LambdaForm classes are also not included.
> oop class_loader = _loader_data->class_loader();
> ResourceMark rm(THREAD);
> bool skip = false;
> if (class_loader == NULL || \
> SystemDictionary::is_platform_class_loader(class_loader)) { // For the boot and \
> platform class loaders, skip classes that are not found in the // java runtime \
> image, such as those found in the --patch-module entries. // These classes can't be \
> loaded from the archive during runtime. if \
> (!ClassLoader::is_modules_image(stream->source()) && strncmp(stream->source(), \
> "jrt:", 4) != 0) { skip = true;
> }
>
> if (class_loader == NULL && ClassLoader::contains_append_entry(stream->source())) {
> // .. but don't skip the boot classes that are loaded from -Xbootclasspath/a
> // as they can be loaded from the archive during runtime.
> skip = false;
> }
> }
> if (skip) {
> tty->print_cr("skip writing class %s from source %s to classlist file",
> _class_name->as_C_string(), stream->source());
> } else {
> classlist_file->print_cr("%s", _class_name->as_C_string());
> classlist_file->flush();
> }
> }
> }
>
>
> This message is printed at run time for the exploded build:
>
> Java HotSpot(TM) 64-Bit Server VM warning: DumpLoadedClassList and CDS are not \
> supported in exploded build
> What do you think?
Looks good.
Thanks,
Jiangli
>
> - Ioi
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic