Re: RFR: 8268821: Split systemDictionaryShared.cpp [v3]
On Thu, 24 Jun 2021 03:11:35 GMT, Yumin Qi wrote: >> Hi, Please review >> systemDictionaryShared becomes fatter and fatter so it is time to split it >> into functional files. Moved security and jar operation related code into >> CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to >> sharedClassInfo.[ch]pp, also moved lambda proxy related to >> lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and >> light. >> >> Tests: tier1,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Reorder including header files, remove unused function > dd_to_dump_time_lambda_proxy_class_dictionary > > _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) > > on [hotspot-runtime-dev](mailto:hotspot-runtime-...@mail.openjdk.java.net):_ > > On 24/06/2021 2:23 am, Yumin Qi wrote: > > > On Wed, 23 Jun 2021 06:08:41 GMT, Yumin Qi wrote: > > > > Hi, Please review > > > > systemDictionaryShared becomes fatter and fatter so it is time to split > > > > it into functional files. Moved security and jar operation related code > > > > into CDSProtectionDomain, and moved shared class info > > > > (DumpTime/RunTime) to sharedClassInfo.[ch]pp, also moved lambda proxy > > > > related to lambdaProxyClassInfo.hpp. This way > > > > systemDictionaryShared.cpp looks neat and light. > > > > Tests: tier1,tier3,tier4 > > > > Thanks > > > > Yumin > > > > > > > > > > _Mailing list message from [David Holmes](mailto:david.holmes at > > > > oracle.com) on [build-dev](mailto:build-dev at mail.openjdk.java.net):_ > > > > Hi Yumin, > > > > On 23/06/2021 4:19 pm, Yumin Qi wrote: > > > > > Hi, Please review > > > > > systemDictionaryShared becomes fatter and fatter so it is time to > > > > > split it into functional files. Moved security and jar operation > > > > > related code into CDSProtectionDomain, and moved shared class info > > > > > (DumpTime/RunTime) to sharedClassInfo.[ch]pp, also moved lambda proxy > > > > > related to lambdaProxyClassInfo.hpp. This way > > > > > systemDictionaryShared.cpp looks neat and light. > > > > > > > > > > > > I'm not really seeing a consistent or recognisable naming pattern here. > > > > We seem to have a mix of: > > > > > > > > * cds/foo.cpp > > > > * cds/fooShared.cpp > > > > * cds/sharedFoo.cpp > > > > Can we establish a simple naming scheme here? > > > > Thanks, > > > > David > > > > > > > > > Thanks David. I was thinking of that too. The best practice is for a > > > class Foo we have foo.hpp for definition and foo.cpp for implementation. > > > Here indeed exists non-consistency that I put DumpTime/RunTtime in a > > > single file. Let me double check and update. > > > > > > That's not what I was saying. I'm talking about the names of the cpp > > file and whether they contain "shared" and whether it is a prefix or > > postfix. There doesn't seem to be a consistent naming scheme employed here. > > That comes from day one. The case class FooShared is like > cds/fooShared.[ch]pp > Usually it is for a class with counterpart of a non-shared version, like > Metaspace, SystemDictionary etc. > The classes [DumpTime/RunTime]SharedClassInfo are used for shared only, they > don't have non-shared version. The "Shared" embedded just an indication of > used in CDS, without it is OK i think. I agree that it's fine to drop the word "Shared" from the [DumpTime/RunTime]SharedClassInfo so the the new files could be named [dump|run]TimeClassInfo.[c|h]pp instead. - PR: https://git.openjdk.java.net/jdk/pull/4568
Re: RFR: 8268821: Split systemDictionaryShared.cpp [v3]
> Hi, Please review > systemDictionaryShared becomes fatter and fatter so it is time to split it > into functional files. Moved security and jar operation related code into > CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to > sharedClassInfo.[ch]pp, also moved lambda proxy related to > lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and > light. > > Tests: tier1,tier3,tier4 > > Thanks > Yumin Yumin Qi has updated the pull request incrementally with one additional commit since the last revision: Reorder including header files, remove unused function dd_to_dump_time_lambda_proxy_class_dictionary - Changes: - all: https://git.openjdk.java.net/jdk/pull/4568/files - new: https://git.openjdk.java.net/jdk/pull/4568/files/8d104acf..438d54da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4568=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4568=01-02 Stats: 11 lines in 4 files changed: 4 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4568.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4568/head:pull/4568 PR: https://git.openjdk.java.net/jdk/pull/4568
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v7]
On Wed, 23 Jun 2021 00:31:55 GMT, Scott Gibbons wrote: >> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration. >> Also allows for performance improvement for non-AVX-512 enabled platforms. >> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to >> accept an additional parameter (isMIME) for fast-path MIME decoding. >> >> A change was made to the signature of DecodeBlock in Base64.java to provide >> the intrinsic information as to whether MIME decoding was being done. This >> allows for the intrinsic to bypass the expensive setup of zmm registers from >> AVX tables, knowing there may be invalid Base64 characters every 76 >> characters or so. A change was also made here removing the restriction that >> the intrinsic must return an even multiple of 3 bytes decoded. This >> implementation handles the pad characters at the end of the string and will >> return the actual number of characters decoded. >> >> The AVX portion of this code will decode in blocks of 256 bytes per loop >> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The >> non-AVX code is an assembly-optimized version of the java DecodeBlock and >> behaves identically. >> >> Running the Base64Decode benchmark, this change increases decode performance >> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers >> are given in the table below. >> >> **Base Score** is without intrinsic support, **Optimized Score** is using >> this intrinsic, and **Gain** is **Base** / **Optimized**. >> >> >> Benchmark Name | Base Score | Optimized Score | Gain >> -- | -- | -- | -- >> testBase64Decode size 1 | 15.36 | 15.32 | 1.00 >> testBase64Decode size 3 | 17.00 | 16.72 | 1.02 >> testBase64Decode size 7 | 20.60 | 18.82 | 1.09 >> testBase64Decode size 32 | 34.21 | 26.77 | 1.28 >> testBase64Decode size 64 | 54.43 | 38.35 | 1.42 >> testBase64Decode size 80 | 66.40 | 48.34 | 1.37 >> testBase64Decode size 96 | 73.16 | 52.90 | 1.38 >> testBase64Decode size 112 | 84.93 | 51.82 | 1.64 >> testBase64Decode size 512 | 288.81 | 32.04 | 9.01 >> testBase64Decode size 1000 | 560.48 | 40.79 | 13.74 >> testBase64Decode size 2 | 9530.28 | 483.37 | 19.72 >> testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15 >> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07 >> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10 >> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02 >> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10 >> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05 >> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00 >> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05 >> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20 >> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09 >> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12 >> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09 >> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21 >> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29 >> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12 >> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05 >> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18 >> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02 >> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24 >> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23 >> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24 >> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14 >> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19 >> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23 >> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing Windows build warnings I will run our internal testing before approving this. - PR: https://git.openjdk.java.net/jdk/pull/4368
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8266854: LibraryCallKit::inline_preconditions_checkIndex modifies control flow even if the intrinsic bailed out - 8254571: Erroneous generic type inference in a lambda expression with a checked exception - 8269125: Klass enqueue element size calculation wrong when traceid value cross compress limit - 8268961: Parenthesized pattern with guards does not work - 8269066: assert(ZAddress::is_marked(addr)) failed: Should be marked - 8269064: Dropped messages of AsyncLogWriter cause memleak - 8269148: Update minor GCC version in GitHub Actions pipeline - 8266885: [aarch64] Crash with 'Field too big for insn' for some tests under compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/ The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/4579/files Stats: 408 lines in 17 files changed: 347 ins; 32 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/4579.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4579/head:pull/4579 PR: https://git.openjdk.java.net/jdk/pull/4579
Re: RFR: 8268821: Split systemDictionaryShared.cpp [v2]
On Wed, 23 Jun 2021 20:45:49 GMT, Yumin Qi wrote: >> Hi, Please review >> systemDictionaryShared becomes fatter and fatter so it is time to split it >> into functional files. Moved security and jar operation related code into >> CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to >> sharedClassInfo.[ch]pp, also moved lambda proxy related to >> lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and >> light. >> >> Tests: tier1,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Split further sharedClassInfo.hpp to dumpTimeSharedClassInfo.hpp and > runTimeSharedClassInfo.hpp to conform to simple rule of separation of > definition and implementation > _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on > [hotspot-runtime-dev](mailto:hotspot-runtime-...@mail.openjdk.java.net):_ > > On 24/06/2021 2:23 am, Yumin Qi wrote: > > > On Wed, 23 Jun 2021 06:08:41 GMT, Yumin Qi wrote: > > > Hi, Please review > > > systemDictionaryShared becomes fatter and fatter so it is time to split > > > it into functional files. Moved security and jar operation related code > > > into CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) > > > to sharedClassInfo.[ch]pp, also moved lambda proxy related to > > > lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat > > > and light. > > > Tests: tier1,tier3,tier4 > > > Thanks > > > Yumin > > > > > > > _Mailing list message from [David Holmes](mailto:david.holmes at > > > oracle.com) on [build-dev](mailto:build-dev at mail.openjdk.java.net):_ > > > Hi Yumin, > > > On 23/06/2021 4:19 pm, Yumin Qi wrote: > > > > Hi, Please review > > > > systemDictionaryShared becomes fatter and fatter so it is time to split > > > > it into functional files. Moved security and jar operation related code > > > > into CDSProtectionDomain, and moved shared class info > > > > (DumpTime/RunTime) to sharedClassInfo.[ch]pp, also moved lambda proxy > > > > related to lambdaProxyClassInfo.hpp. This way > > > > systemDictionaryShared.cpp looks neat and light. > > > > > > > > > I'm not really seeing a consistent or recognisable naming pattern here. > > > We seem to have a mix of: > > > - cds/foo.cpp > > > - cds/fooShared.cpp > > > - cds/sharedFoo.cpp > > > Can we establish a simple naming scheme here? > > > Thanks, > > > David > > > > > > Thanks David. I was thinking of that too. The best practice is for a class > > Foo we have foo.hpp for definition and foo.cpp for implementation. Here > > indeed exists non-consistency that I put DumpTime/RunTtime in a single > > file. Let me double check and update. > > That's not what I was saying. I'm talking about the names of the cpp > file and whether they contain "shared" and whether it is a prefix or > postfix. There doesn't seem to be a consistent naming scheme employed here. That comes from day one. The case class FooShared is like cds/fooShared.[ch]pp Usually it is for a class with counterpart of a non-shared version, like Metaspace, SystemDictionary etc. The classes [DumpTime/RunTime]SharedClassInfo are used for shared only, they don't have non-shared version. The "Shared" embedded just an indication of used in CDS, without it is OK i think. - PR: https://git.openjdk.java.net/jdk/pull/4568
Re: RFR: 8268821: Split systemDictionaryShared.cpp [v2]
On Wed, 23 Jun 2021 20:45:49 GMT, Yumin Qi wrote: >> Hi, Please review >> systemDictionaryShared becomes fatter and fatter so it is time to split it >> into functional files. Moved security and jar operation related code into >> CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to >> sharedClassInfo.[ch]pp, also moved lambda proxy related to >> lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and >> light. >> >> Tests: tier1,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Split further sharedClassInfo.hpp to dumpTimeSharedClassInfo.hpp and > runTimeSharedClassInfo.hpp to conform to simple rule of separation of > definition and implementation Looks good. Few minor comments. src/hotspot/share/cds/cdsProtectionDomain.cpp line 34: > 32: #include "classfile/vmClasses.hpp" > 33: #include "classfile/vmSymbols.hpp" > 34: #include "classfile/symbolTable.hpp" Please move line 34 before line 31. src/hotspot/share/cds/dumpTimeSharedClassInfo.hpp line 31: > 29: #include "cds/archiveBuilder.hpp" > 30: #include "cds/archiveUtils.hpp" > 31: #include "cds/metaspaceShared.hpp" Please move line 28 after line 31. src/hotspot/share/cds/metaspaceShared.cpp line 35: > 33: #include "cds/lambdaFormInvokers.hpp" > 34: #include "cds/metaspaceShared.hpp" > 35: #include "cds/cdsProtectionDomain.hpp" This line should be after line 27. src/hotspot/share/classfile/systemDictionaryShared.hpp line 291: > 289: static void start_dumping() NOT_CDS_RETURN; > 290: static bool is_supported_invokedynamic(BootstrapInfo* bsi) > NOT_CDS_RETURN_(false); > 291: static void > dd_to_dump_time_lambda_proxy_class_dictionary(LambdaProxyClassKey key, > InstanceKlass* proxy_klass); I don't think this is being referenced. - Changes requested by ccheung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4568
RE: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v7]
Hi, David. I don't have permissions to run tests in this repo. I have tested on several x86 platforms (ICX, SKL) with several options. I'll be running more tests today. Thanks, --Scott -Original Message- From: hotspot-dev On Behalf Of David Holmes Sent: Tuesday, June 22, 2021 7:21 PM To: build-dev@openjdk.java.net; core-libs-...@openjdk.java.net; hotspot-...@openjdk.java.net; hotspot-compiler-...@openjdk.java.net Subject: Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v7] On Wed, 23 Jun 2021 00:31:55 GMT, Scott Gibbons wrote: >> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration. >> Also allows for performance improvement for non-AVX-512 enabled platforms. >> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to >> accept an additional parameter (isMIME) for fast-path MIME decoding. >> >> A change was made to the signature of DecodeBlock in Base64.java to provide >> the intrinsic information as to whether MIME decoding was being done. This >> allows for the intrinsic to bypass the expensive setup of zmm registers from >> AVX tables, knowing there may be invalid Base64 characters every 76 >> characters or so. A change was also made here removing the restriction that >> the intrinsic must return an even multiple of 3 bytes decoded. This >> implementation handles the pad characters at the end of the string and will >> return the actual number of characters decoded. >> >> The AVX portion of this code will decode in blocks of 256 bytes per loop >> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The >> non-AVX code is an assembly-optimized version of the java DecodeBlock and >> behaves identically. >> >> Running the Base64Decode benchmark, this change increases decode performance >> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers >> are given in the table below. >> >> **Base Score** is without intrinsic support, **Optimized Score** is using >> this intrinsic, and **Gain** is **Base** / **Optimized**. >> >> >> Benchmark Name | Base Score | Optimized Score | Gain >> -- | -- | -- | -- >> testBase64Decode size 1 | 15.36 | 15.32 | 1.00 testBase64Decode size >> 3 | 17.00 | 16.72 | 1.02 testBase64Decode size 7 | 20.60 | 18.82 | >> 1.09 testBase64Decode size 32 | 34.21 | 26.77 | 1.28 testBase64Decode >> size 64 | 54.43 | 38.35 | 1.42 testBase64Decode size 80 | 66.40 | >> 48.34 | 1.37 testBase64Decode size 96 | 73.16 | 52.90 | 1.38 >> testBase64Decode size 112 | 84.93 | 51.82 | 1.64 testBase64Decode >> size 512 | 288.81 | 32.04 | 9.01 testBase64Decode size 1000 | 560.48 >> | 40.79 | 13.74 testBase64Decode size 2 | 9530.28 | 483.37 | >> 19.72 testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15 >> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07 >> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10 >> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02 >> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10 >> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05 >> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00 >> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05 >> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20 >> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09 >> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12 >> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09 >> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21 >> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29 >> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12 >> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05 >> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18 >> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02 >> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24 >> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23 >> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24 >> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14 >> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19 >> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23 >> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing Windows build warnings What testing has been done for this change? I do not see that the Github Actions have been run for this PR. Has this been tested on a range of x86 systems with differing AVX capabilities? Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/4368
Re: RFR: 8268821: Split systemDictionaryShared.cpp [v2]
> Hi, Please review > systemDictionaryShared becomes fatter and fatter so it is time to split it > into functional files. Moved security and jar operation related code into > CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to > sharedClassInfo.[ch]pp, also moved lambda proxy related to > lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and > light. > > Tests: tier1,tier3,tier4 > > Thanks > Yumin Yumin Qi has updated the pull request incrementally with one additional commit since the last revision: Split further sharedClassInfo.hpp to dumpTimeSharedClassInfo.hpp and runTimeSharedClassInfo.hpp to conform to simple rule of separation of definition and implementation - Changes: - all: https://git.openjdk.java.net/jdk/pull/4568/files - new: https://git.openjdk.java.net/jdk/pull/4568/files/e3cd258f..8d104acf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4568=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4568=00-01 Stats: 1328 lines in 11 files changed: 713 ins; 611 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4568.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4568/head:pull/4568 PR: https://git.openjdk.java.net/jdk/pull/4568
Re: RFR: 8268821: Split systemDictionaryShared.cpp
On Wed, 23 Jun 2021 06:08:41 GMT, Yumin Qi wrote: > Hi, Please review > systemDictionaryShared becomes fatter and fatter so it is time to split it > into functional files. Moved security and jar operation related code into > CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to > sharedClassInfo.[ch]pp, also moved lambda proxy related to > lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and > light. > > Tests: tier1,tier3,tier4 > > Thanks > Yumin > _Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on > [build-dev](mailto:build-...@mail.openjdk.java.net):_ > > Hi Yumin, > > On 23/06/2021 4:19 pm, Yumin Qi wrote: > > > Hi, Please review > > systemDictionaryShared becomes fatter and fatter so it is time to split it > > into functional files. Moved security and jar operation related code into > > CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to > > sharedClassInfo.[ch]pp, also moved lambda proxy related to > > lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat > > and light. > > I'm not really seeing a consistent or recognisable naming pattern here. > We seem to have a mix of: > > - cds/foo.cpp > - cds/fooShared.cpp > - cds/sharedFoo.cpp > > Can we establish a simple naming scheme here? > > Thanks, > David Thanks David. I was thinking of that too. The best practice is for a class Foo we have foo.hpp for definition and foo.cpp for implementation. Here indeed exists non-consistency that I put DumpTime/RunTtime in a single file. Let me double check and update. Yumin - PR: https://git.openjdk.java.net/jdk/pull/4568
Re: RFR: 8268821: Split systemDictionaryShared.cpp
On Wed, 23 Jun 2021 12:40:57 GMT, Erik Joelsson wrote: >> Hi, Please review >> systemDictionaryShared becomes fatter and fatter so it is time to split it >> into functional files. Moved security and jar operation related code into >> CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to >> sharedClassInfo.[ch]pp, also moved lambda proxy related to >> lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and >> light. >> >> Tests: tier1,tier3,tier4 >> >> Thanks >> Yumin > > Build changes look ok. @erikj79 Thanks for review! - PR: https://git.openjdk.java.net/jdk/pull/4568
Re: RFR: 8268821: Split systemDictionaryShared.cpp
On Wed, 23 Jun 2021 06:08:41 GMT, Yumin Qi wrote: > Hi, Please review > systemDictionaryShared becomes fatter and fatter so it is time to split it > into functional files. Moved security and jar operation related code into > CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to > sharedClassInfo.[ch]pp, also moved lambda proxy related to > lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and > light. > > Tests: tier1,tier3,tier4 > > Thanks > Yumin Build changes look ok. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4568
[jdk17] Integrated: 8269148: Update minor GCC version in GitHub Actions pipeline
On Tue, 22 Jun 2021 17:20:14 GMT, Aleksey Shipilev wrote: > It seems Ubuntu had bumped the version for GCC, so GHA started to fail with > e.g.: > > > The following packages have unmet dependencies: > g++-10-s390x-linux-gnu : Depends: gcc-10-s390x-linux-gnu-base (= > 10.2.0-5ubuntu1~20.04cross1) but 10.3.0-1ubuntu1~20.04cross1 is to be > installed > gcc-10-s390x-linux-gnu : Depends: cpp-10-s390x-linux-gnu (= > 10.2.0-5ubuntu1~20.04cross1) but 10.3.0-1ubuntu1~20.04cross1 is to be > installed > Depends: gcc-10-s390x-linux-gnu-base (= > 10.2.0-5ubuntu1~20.04cross1) but 10.3.0-1ubuntu1~20.04cross1 is to be > installed > E: Unable to correct problems, you have held broken packages. > Error: Process completed with exit code 100. > > > I believe we should just update to `10.3.0-1ubuntu1~20.04`. > > Additional testing: > - [x] GitHub Actions, package installation steps work > - [x] GitHub Actions, the builds complete This pull request has now been integrated. Changeset: ce917b23 Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk17/commit/ce917b23596415ab48f95f36c7d88adb1ec0df68 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod 8269148: Update minor GCC version in GitHub Actions pipeline Reviewed-by: erikj, dholmes, xliu - PR: https://git.openjdk.java.net/jdk17/pull/120
Re: [jdk17] RFR: 8269148: Update minor GCC version in GitHub Actions pipeline
On Tue, 22 Jun 2021 17:20:14 GMT, Aleksey Shipilev wrote: > It seems Ubuntu had bumped the version for GCC, so GHA started to fail with > e.g.: > > > The following packages have unmet dependencies: > g++-10-s390x-linux-gnu : Depends: gcc-10-s390x-linux-gnu-base (= > 10.2.0-5ubuntu1~20.04cross1) but 10.3.0-1ubuntu1~20.04cross1 is to be > installed > gcc-10-s390x-linux-gnu : Depends: cpp-10-s390x-linux-gnu (= > 10.2.0-5ubuntu1~20.04cross1) but 10.3.0-1ubuntu1~20.04cross1 is to be > installed > Depends: gcc-10-s390x-linux-gnu-base (= > 10.2.0-5ubuntu1~20.04cross1) but 10.3.0-1ubuntu1~20.04cross1 is to be > installed > E: Unable to correct problems, you have held broken packages. > Error: Process completed with exit code 100. > > > I believe we should just update to `10.3.0-1ubuntu1~20.04`. > > Additional testing: > - [x] GitHub Actions, package installation steps work > - [x] GitHub Actions, the builds complete I am integrating this to unbreak the testing. - PR: https://git.openjdk.java.net/jdk17/pull/120
Re: RFR: 8268821: Split systemDictionaryShared.cpp
Hi Yumin, On 23/06/2021 4:19 pm, Yumin Qi wrote: Hi, Please review systemDictionaryShared becomes fatter and fatter so it is time to split it into functional files. Moved security and jar operation related code into CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to sharedClassInfo.[ch]pp, also moved lambda proxy related to lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and light. I'm not really seeing a consistent or recognisable naming pattern here. We seem to have a mix of: - cds/foo.cpp - cds/fooShared.cpp - cds/sharedFoo.cpp Can we establish a simple naming scheme here? Thanks, David Tests: tier1,tier3,tier4 Thanks Yumin - Commit messages: - 8268821: Split systemDictionaryShared.cpp Changes: https://git.openjdk.java.net/jdk/pull/4568/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4568=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268821 Stats: 2606 lines in 11 files changed: 1310 ins; 1101 del; 195 mod Patch: https://git.openjdk.java.net/jdk/pull/4568.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4568/head:pull/4568 PR: https://git.openjdk.java.net/jdk/pull/4568
RFR: 8268821: Split systemDictionaryShared.cpp
Hi, Please review systemDictionaryShared becomes fatter and fatter so it is time to split it into functional files. Moved security and jar operation related code into CDSProtectionDomain, and moved shared class info (DumpTime/RunTime) to sharedClassInfo.[ch]pp, also moved lambda proxy related to lambdaProxyClassInfo.hpp. This way systemDictionaryShared.cpp looks neat and light. Tests: tier1,tier3,tier4 Thanks Yumin - Commit messages: - 8268821: Split systemDictionaryShared.cpp Changes: https://git.openjdk.java.net/jdk/pull/4568/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4568=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268821 Stats: 2606 lines in 11 files changed: 1310 ins; 1101 del; 195 mod Patch: https://git.openjdk.java.net/jdk/pull/4568.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4568/head:pull/4568 PR: https://git.openjdk.java.net/jdk/pull/4568
