RFR(L) JDK-8188791 Move AppCDS from closed repo to open repo
Volker Simonis
volker.simonis at gmail.com
Tue Nov 28 09:42:41 UTC 2017
On Mon, Nov 27, 2017 at 9:37 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> Hi Volker & Dmitry,
>
> Thanks for testing on the additional platforms. It really is great help!
>
> We had a very limited set of supported platforms for the custom loaders due
> to lack of testing resources. I think now we can expand the set of
> platforms. However, I would like to keep JDK-8188791 as simple as possible
> -- just moving the code, but don't introduce other changes. So I've created
>
> https://bugs.openjdk.java.net/browse/JDK-8191927
>
> Enable AppCDS for custom loaders on all 64-bit Linux and AIX
>
> We can try to push that after 8191927 is pushed (which should happen very
> soon once the JEP is targeted to JDK 10, hopefully today).
>
Thanks for opening the bug. I like your changes to it which introduce
"@requires vm.cds.custom.loaders" to simplify the logic:
http://cr.openjdk.java.net/~iklam/jdk10/8191927-appcds-custom-loader-for-more-platforms.v01/
After you've pushed 8188791 please send out a RFR for 8191927. I can
review it so we can push it in time before jdk10 will be branched off.
Regards,
Volker
> I've posted the latest webrev at
>
> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds.v03/
>
> It's based on this changeset
>
> changeset: 48006:235a18d659fc
> user: roland
> date: Mon Nov 27 10:44:19 2017 -0800
> files: src/hotspot/share/opto/split_if.cpp
> test/hotspot/jtreg/compiler/loopopts/TestSplitIfPinnedCMove.java
> description:
> 8191153: assert(u_ctrl != blk1 && u_ctrl != blk2) failed: won't converge
> Summary: relax assert
> Reviewed-by: kvn
>
> This version also fixes the issue with -XX:+PrintSystemDictionaryAtExit on
> product builds with
> the two tests DumpClassList.java and ProhibitedPackage.java.
>
> Thanks
>
> - Ioi
>
>
>
>
> On 11/27/17 11:15 AM, Dmitry Samersoff wrote:
>>
>> Volker,
>>
>>> @Dmitry: can you please re-build and re-run the tests on 64-bit ARM
>>> just to make sure everything is working? Notice, that with my add-ons
>>> about 20 more tests than before will be executed, but that was no
>>> problem an the other Linux platforms I've tested.
>>
>>
>> I'll do it.
>>
>> -Dmitry
>>
>>
>> On 11/27/2017 09:34 PM, Volker Simonis wrote:
>>>
>>> Hi Ioi, Dmitry,
>>>
>>> I've finished my testing on Linux/ppc64, Linux/ppc64le, Linux/s390 and
>>> AIX. All the tests passed successfully (with the corresponding extra
>>> ppc/s390 patches currently under review).
>>>
>>> I did run the tests with the following add-on to your change, which
>>> enables AppCDS with custom class loaders on all 64-bit Linux, Solaris
>>> and AIX platforms:
>>>
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2017/8188791-open-appcds.v02.tests-addon/
>>>
>>> The only change required in HotSpot is in "classListParser.cpp" to
>>> enable the feature on the named platforms. The other changes just
>>> enable the additional tests on all 64-bit Linux, Solaris and AIX
>>> platfroms.
>>>
>>> @Ioi: can you please append this add-on to your change?
>>>
>>> @Dmitry: can you please re-build and re-run the tests on 64-bit ARM
>>> just to make sure everything is working? Notice, that with my add-ons
>>> about 20 more tests than before will be executed, but that was no
>>> problem an the other Linux platforms I've tested.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>>
>>> On Fri, Nov 24, 2017 at 5:08 PM, Dmitry Samersoff <dms at samersoff.net>
>>> wrote:
>>>>
>>>> Volker,
>>>>
>>>>> Currently I'm building and running some tests
>>>>> and I hope that until Monday we may even extend the list of supported
>>>>> platforms by Linux/s390. I'm not sure about AArch (Dmitry?), but if
>>>>> that works as well, the check could be simplified to allow all the
>>>>> 64-bit Linux platforms.
>>>>
>>>> Yes, I added similar patch for Linux/AARCH64 and it works.
>>>>
>>>> So we should simplify the check.
>>>>
>>>> -Dmitry
>>>>
>>>> On 11/24/2017 05:51 PM, Volker Simonis wrote:
>>>>>
>>>>> Hi Ioi,
>>>>>
>>>>> you patch is not applying any more after "8187118: Remove appending
>>>>> -cp path to the boot class path at AppCDS dump time" was pushed to
>>>>> jdk/hs because of conflicts in classLoaderExt.hpp.
>>>>>
>>>>> Could you please rebase your change upon the newest version of jdk/hs ?
>>>>>
>>>>> By the way, on ppc64 all the tests passed after I found and fixed a
>>>>> trivial bug (I've opened "JDK-8191770: [ppc64] Fix CDS: don't rewrite
>>>>> invokefinal if DumpSharedSpaces" for it).
>>>>>
>>>>> Can you please explain why AppCDS with custom class loaders is
>>>>> currently restricted to Linux/x86_64 and 64-bit Solaris?
>>>>>
>>>>> With the following small patch:
>>>>>
>>>>> diff -r 23deffd3a9c4 src/hotspot/share/classfile/classListParser.cpp
>>>>> --- a/src/hotspot/share/classfile/classListParser.cpp Fri Nov 24
>>>>> 15:37:19 2017 +0100
>>>>> +++ b/src/hotspot/share/classfile/classListParser.cpp Fri Nov 24
>>>>> 15:38:21 2017 +0100
>>>>> @@ -272,6 +272,7 @@
>>>>> // during archive dumping.
>>>>> InstanceKlass* ClassListParser::load_class_from_source(Symbol*
>>>>> class_name, TRAPS) {
>>>>> #if !((defined(LINUX) && defined(X86) && defined(_LP64)) || \
>>>>> + (defined(PPC) && defined(_LP64)) || \
>>>>> (defined(SOLARIS) && defined(_LP64)))
>>>>> // The only supported platforms are: (1) Linux/AMD64; (2)
>>>>> Solaris/64-bit
>>>>> error("AppCDS custom class loaders not supported on this
>>>>> platform");
>>>>> diff -r 23deffd3a9c4
>>>>>
>>>>> test/hotspot/jtreg/runtime/appcds/customLoader/UnsupportedPlatforms.java
>>>>> ---
>>>>> a/test/hotspot/jtreg/runtime/appcds/customLoader/UnsupportedPlatforms.java
>>>>> Fri Nov 24 15:37:19 2017 +0100
>>>>> +++
>>>>> b/test/hotspot/jtreg/runtime/appcds/customLoader/UnsupportedPlatforms.java
>>>>> Fri Nov 24 15:38:21 2017 +0100
>>>>> @@ -56,6 +56,7 @@
>>>>> OutputAnalyzer out = TestCommon.dump(appJar, classlist);
>>>>>
>>>>> if ((Platform.isSolaris() && Platform.is64bit()) ||
>>>>> + (Platform.isLinux() && Platform.isPPC()) ||
>>>>> (Platform.isLinux() && Platform.isX64())) {
>>>>> out.shouldNotContain(PLATFORM_NOT_SUPPORTED_WARNING);
>>>>> out.shouldHaveExitValue(0);
>>>>>
>>>>>
>>>>> I could at least pass all the relevant jtreg tests on Linux/ppc64. So
>>>>> if there are no other hidden reasons I'd kindly ask you to add this
>>>>> patch to you change. I'm currently testing on AIX and I hope the tests
>>>>> will also succeed. If this will be the case, the list in the
>>>>> aforementioned patch could also be extended by AIX.
>>>>>
>>>>> Finally I've also did some small fixes on s390 (opened "8191863:
>>>>> [s390] Fix CDS: some bytecode rewriting doesn't depend on
>>>>> RewriteControl" for it). Currently I'm building and running some tests
>>>>> and I hope that until Monday we may even extend the list of supported
>>>>> platforms by Linux/s390. I'm not sure about AArch (Dmitry?), but if
>>>>> that works as well, the check could be simplified to allow all the
>>>>> 64-bit Linux platforms.
>>>>>
>>>>> Thank you and best regards,
>>>>> Volker
>>>>>
>>>>> On Fri, Nov 17, 2017 at 4:51 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>>>>>
>>>>>> Hi Volker,
>>>>>>
>>>>>> Thanks for trying the patch out.
>>>>>>
>>>>>>
>>>>>> On 11/17/17 5:42 AM, Volker Simonis wrote:
>>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>>
>>>>>> I've updated the patch inside the webrev. You can download it from
>>>>>>
>>>>>>
>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds.v02/open.patch
>>>>>>
>>>>>>
>>>>>> The previous patch (which you had the problems with) is still
>>>>>> available as
>>>>>>
>>>>>>
>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8188791-open-appcds.v02/open.patch.old
>>>>>>
>>>>>> I will post a new webrev, but it's big and I might be running out of
>>>>>> quota
>>>>>> from cr.openjdk.java.net, so I need to figure out how to handle that.
>>>>>>
>>>>>> The jdk/hs changeset corresponding to the new patch is the following:
>>>>>>
>>>>>> changeset: 47860:564882d918d4
>>>>>> user: zgu
>>>>>> date: Thu Nov 16 20:21:11 2017 -0500
>>>>>> files: src/hotspot/share/services/memTracker.cpp
>>>>>> description:
>>>>>> 8190357: NMT: Include metadata information in NMT final report when
>>>>>> PrintNMTStatistics is on
>>>>>> Summary: Include metadata information in NMT final report
>>>>>> Reviewed-by: adinn, stuefe
>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> Those are also due to merge issues. With the updated patch, I passed
>>>>>> all
>>>>>> tests under on Linux/x64:
>>>>>>
>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile
>>>>>> test/hotspot/jtreg/runtime/CDSCompressedKPtrs
>>>>>> test/hotspot/jtreg/runtime/modules/PatchModule/PatchModuleCDS.java
>>>>>> test/hotspot/jtreg/runtime/appcds
>>>>>>
>>>>>> $ jtreg -J-Djavatest.maxOutputSize=1000000 -conc:28
>>>>>> -testjdk:/home/iklam/jdk/bld/opencdso-fastdebug/images/jdk
>>>>>> -compilejdk:/home/iklam/jdk/bld/opencds/images/jdk -verbose:2
>>>>>> -timeout:4.0
>>>>>> -retain -agentvm
>>>>>>
>>>>>> -exclude:/home/iklam/jdk/opencds/closed/test/hotspot/jtreg/ProblemList.txt
>>>>>>
>>>>>> -exclude:/home/iklam/jdk/opencds/open/test/hotspot/jtreg/ProblemList.txt
>>>>>>
>>>>>> -nativepath:/home/iklam/jdk/bld/opencdso/images/jdk/../../images/test/hotspot/jtreg/native
>>>>>> -nr -w /home/iklam/tmp/jtreg/work -r /home/iklam/tmp/jtreg/report/
>>>>>> open/test/hotspot/jtreg/runtime/SharedArchiveFile
>>>>>> open/test/hotspot/jtreg/runtime/CDSCompressedKPtrs
>>>>>>
>>>>>> open/test/hotspot/jtreg/runtime/modules/PatchModule/PatchModuleCDS.java
>>>>>> open/test/hotspot/jtreg/runtime/appcds
>>>>>> [...]
>>>>>> Test results: passed: 128
>>>>>> Results written to /jdk/tmp/jtreg/work
>>>>>>
>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>>
>>>>>> You're correct. I will exclude those tests from product builds, and
>>>>>> file an
>>>>>> RFE to fix them later.
>>>>>>
>>>>>> Thanks
>>>>>> - Ioi
>>>>>>
>>>>>>> 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