RFR(L) JDK-8188791 Move AppCDS from closed repo to open repo

Volker Simonis volker.simonis at gmail.com
Fri Nov 17 13:42:37 UTC 2017


Hi Ioi,

I'm just trying to verify if AppCDS will be running on our ppc/s390/AIX ports.

I applied your patch to the current jdk-hs repo as of today and first
of all I see the same compilation problems already reported by Dmitry
before. Can you please update you webrev with the corresponding
changes or create a new one. That will help other who wish to look at
this change.

After building, I ran the appcds jtreg tests (i.e.
test/hotspot/jtreg/runtime/appcds/) under Linux/x86_64 and again I see
the same four failures like Dmitry.

Is that expected? I see a lot more errors on Linux/ppc64le but before
going into more detail I'd like to know what is the correct baseline
on which we can rely.

Finally, I saw that with a product build, I get two additional test failures:

runtime/appcds/ProhibitedPackage.java
runtime/appcds/DumpClassList.java

because of:

Error: VM option 'PrintSystemDictionaryAtExit' is notproduct and is
available only in debug version of VM.

So these two tests should probably be adjusted to only run for
slow/fastdebug builds.

Thank you and best regards,
Volker


On Thu, Nov 16, 2017 at 12:22 PM, Dmitry Samersoff <dms at samersoff.net> wrote:
> Ioi,
>
> Thank you!
>
> I did some additional testing and find that four tests
> fails the same way on both x86_64 and AARCH64 (see below).
>
> runtime/appcds/VerifierTest_0.java
> runtime/appcds/cacheObject/GCStressTest.java
> runtime/appcds/cacheObject/RedefineClassTest.java
> runtime/appcds/sharedStrings/InternSharedString.java
>
> Is it expected?
>
> test result: Failed. Execution failed: `main' threw exception:
> java.lang.RuntimeException: 'Please remove the unverifiable cl
> asses' missing from stdout/stderr
>
> Exception in thread "main" java.lang.RuntimeException: FAILED.
> GCStressApp_nonshared_string should not be shared
>         at GCStressApp.main(GCStressApp.java:73)
>
>
> FAILED. buzzshould not be shared
>
> Exception in thread "main" java.lang.RuntimeException: Failed:
> unexpected shared string.
>         at InternStringTest.main(InternStringTest.java:63)
>
> -Dmitry
>
> On 16.11.2017 03:10, Ioi Lam wrote:
>> I filed:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8191375 Add high-level jtreg
>> VMProps to filter out CDS tests
>>
>> https://bugs.openjdk.java.net/browse/JDK-8191374 Improve error message
>> when CDS is not supported
>>
>> Thanks
>>
>> - Ioi
>>
>>
>>
>> On 11/15/17 4:01 PM, Ioi Lam wrote:
>>> Hi Dmitry,
>>>
>>> Thanks for the review.
>>>
>>> On 11/6/17 7:07 AM, Dmitry Samersoff wrote:
>>>
>>>> Ioi,
>>>>
>>>> I tested new patch on aarch64 and it works fine with the changes
>>>> below[1].
>>> I've fixed it. Thanks for the patch.
>>>> Also tests doesn't work with exploded image - is it possible to check
>>>> for this case and produce appropriate error message?
>>> CDS is not supported on exploded images. I think the best thing to do
>>> is to add something like "@requires vm.cds" into the test cases
>>> (similar to the use of "@require vm.aot" in other tests). That way,
>>> none of the CDS tests will be executed for
>>>
>>> It's too late to do this in JDK 10 now, but I will file an RFE to do
>>> it in the next release.
>>>
>>> I'll also file another RFE to print a better message. Today if you use
>>> an exploded build the error message is kind of confusing:
>>>
>>> $ ./jdk/bin/java -Xshare:dump
>>> Error: non-empty directory '/jdk/bld/cons/jdk/modules/java.base'
>>> Hint: enable -Xlog:class+path=info to diagnose the failure
>>> Error occurred during initialization of VM
>>> CDS allows only empty directories in archived classpaths
>>>
>>> It should say something like "CDS is not supported on exploded images".
>>>
>>> Thanks
>>> - Ioi
>>>
>>>> 1.
>>>> diff -r dbf9cec6a568 src/hotspot/share/classfile/classListParser.cpp
>>>> --- a/src/hotspot/share/classfile/classListParser.cpp   Mon Nov 06
>>>> 09:45:23 2017 +0000
>>>> +++ b/src/hotspot/share/classfile/classListParser.cpp   Mon Nov 06
>>>> 15:02:51 2017 +0000
>>>> @@ -31,7 +31,6 @@
>>>> -#include "prims/jvm.h"
>>>>
>>>> diff -r dbf9cec6a568
>>>> src/hotspot/share/classfile/systemDictionaryShared.cpp
>>>> --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp Mon Nov
>>>> 06 09:45:23 2017 +0000
>>>> +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp Mon Nov
>>>> 06 15:02:51 2017 +0000
>>>> @@ -518,7 +518,7 @@
>>>>
>>>>       {
>>>>         MutexLocker mu(SystemDictionary_lock, THREAD);
>>>> -      Klass* check = find_class(d_index, d_hash, name, dictionary);
>>>> +      Klass* check = dictionary->find_class(d_index, d_hash, name);
>>>>         if (check != NULL) {
>>>>           return InstanceKlass::cast(check);
>>>>         }
>>>>
>>>> -Dmitry
>>>>
>>>> On 11/01/2017 05:43 AM, Ioi Lam wrote:
>>>>> Hi,
>>>>>
>>>>> Here's the new webrev for both the AppCDS implementation and tests.
>>>>> During internal review of the JEP, we have decided to integrate both
>>>>> implementation and tests at the same time.
>>>>>
>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds.v02/
>>>>>
>>>>> As mentioned before, most of the "diffs" shown in this webrev are the
>>>>> result of copying the closed source files on top of files of the same
>>>>> name in the open repo. So in reviewing, instead of focusing on what's
>>>>> "changed", please focus on the entire content of the new version of
>>>>> each
>>>>> file.
>>>>>
>>>>> Testing: I did an OpenJDK linux-x64 build (without Oracle closed
>>>>> sources) and all the new appcds tests passed.
>>>>>
>>>>> Thanks
>>>>>
>>>>> - Ioi
>>>>>
>>>>>
>>>>> On 10/30/17 8:52 AM, Ioi Lam wrote:
>>>>>> Hi Dmitry,
>>>>>>
>>>>>> In the latest JDK 10 repo, is_jrt has been renamed to
>>>>>> is_modules_image. Please change the code accordingly.
>>>>>>
>>>>>> I will post my latest diff soon, with some test cases as well.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> - Ioi
>>>>>>
>>>>>>
>>>>>> On 10/30/17 4:04 AM, Dmitry Samersoff wrote:
>>>>>>> Ioi,
>>>>>>>
>>>>>>> I'd tried to apply your patch to latest open JDK10 and
>>>>>>> the compilation fails with:
>>>>>>>
>>>>>>> /root/dsamersoff/ESC/appcds/hs/src/hotspot/share/classfile/systemDictionaryShared.cpp:400:16:
>>>>>>>
>>>>>>>
>>>>>>> error: ‘class SharedClassPathEntry’ has no member named ‘is_jrt’
>>>>>>>
>>>>>>> Did I miss something?
>>>>>>>
>>>>>>> -Dmitry
>>>>>>>
>>>>>>> On 13.10.2017 02:48, Ioi Lam wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please review this change set.
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds-impl.v01/
>>>>>>>>
>>>>>>>>       https://bugs.openjdk.java.net/browse/JDK-8188791
>>>>>>>>
>>>>>>>> This is the first step of implementing the following JEP, which
>>>>>>>> moves
>>>>>>>> AppCDS from
>>>>>>>> closed repos into the openjdk repo:
>>>>>>>>
>>>>>>>>       https://bugs.openjdk.java.net/browse/JDK-8185996
>>>>>>>>
>>>>>>>> In JDK 9, significant portion of AppCDS code resided in the closed
>>>>>>>> repo.
>>>>>>>> As part
>>>>>>>> of the open-sourcing effort of JDK 18.3, we will move the source
>>>>>>>> code
>>>>>>>> into the
>>>>>>>> open repo.
>>>>>>>>
>>>>>>>> In this changeset, the code is moved verbatim as much as
>>>>>>>> possible. The
>>>>>>>> intention is
>>>>>>>> only to relocate the sources, not to changing existing behaviors,
>>>>>>>> and not
>>>>>>>> to do any sort of refactoring.
>>>>>>>>
>>>>>>>> Most of the "diffs" shown in this webrev are the result of
>>>>>>>> copying the
>>>>>>>> closed source
>>>>>>>> files on top of files of the same name in the open repo. So in
>>>>>>>> reviewing, instead of
>>>>>>>> focusing on what's "changed", it's better to focus on the entire
>>>>>>>> content
>>>>>>>> of the new
>>>>>>>> version of each file.
>>>>>>>>
>>>>>>>> The only functional change in this task is that the UseAppCDS
>>>>>>>> flag is
>>>>>>>> changed from
>>>>>>>> a "commercial" flag to a regular "product" flag. This is because
>>>>>>>> "commercial"
>>>>>>>> flags are not supported by the OpenJDK build.
>>>>>>>>
>>>>>>>> Source code refactoring may be desirable, because the old
>>>>>>>> open/closed
>>>>>>>> source
>>>>>>>> code structure had introduced some intermediary APIs to connect code
>>>>>>>> between
>>>>>>>> the two repos. Such API should be removed in a separate RFE.
>>>>>>>>
>>>>>>>> Also, some AppCDS tests are currently in the closed repo. These
>>>>>>>> tests
>>>>>>>> will be
>>>>>>>> moved in a separate task. See JDK-8188792 for details.
>>>>>>>>
>>>>>>>> All the AppCDS tests (currently still in closed sources) passed with
>>>>>>>> both Oracle JDK
>>>>>>>> and OpenJDK.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> - Ioi
>>>
>>
>
>


More information about the hotspot-runtime-dev mailing list