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

[Ioi Lam]

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