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

[Ioi Lam]

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
>