RFR: 8268821: Split systemDictionaryShared.cpp [v3]

Calvin Cheung ccheung at openjdk.java.net
Thu Jun 24 04:21:30 UTC 2021


On Thu, 24 Jun 2021 03:11:35 GMT, Yumin Qi <minqi at openjdk.org> 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.holmes at oracle.com) on [hotspot-runtime-dev](mailto:hotspot-runtime-dev at 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



More information about the build-dev mailing list