RFR(L) JDK-8188791 Move AppCDS from closed repo to open repo
Dmitry Samersoff
dms at samersoff.net
Thu Nov 16 11:22:16 UTC 2017
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