RFR (XS) 8185160 -XX:DumpLoadedClassList omits graal classes
Jiangli Zhou
jiangli.zhou at oracle.com
Fri Oct 20 20:13:38 UTC 2017
> On Oct 20, 2017, at 1:05 PM, Ioi Lam <ioi.lam at 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 at oracle.com <mailto:ioi.lam at 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 at oracle.com <mailto:ioi.lam at 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 at oracle.com <mailto:ioi.lam at 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 at oracle.com <mailto:ioi.lam at 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:
>>>>>>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8185160-dump-class-list-omits-graal.v01/ <http://cr.openjdk.java.net/%7Eiklam/jdk10/8185160-dump-class-list-omits-graal.v01/>
>>>>>>>>>>> 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/test/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
>
>
More information about the hotspot-runtime-dev
mailing list