RFR (XL) 8081519 Split globals.hpp to factor out the Flag class (original) (raw)

Gerard Ziemski gerard.ziemski at oracle.com
Mon Apr 2 14:48:46 UTC 2018

Thank you for the review!

On Mar 31, 2018, at 2:46 AM, David Holmes <david.holmes at oracle.com> wrote:

Hi Gerard, Scanning through this it seems okay. Like Coleen I'm wondering why so many "flag" using .cpp files don't include jvmFlag.hpp? I expect they transitively included globals.hpp previously but now will need an explicit include.

I tested it with precompiled headers and without and all was good and I wanted to make as few changes to the include headers, but will explicitly add the headers.

The two new files have a slight formatting error with their copyright, you need a comma after 2018.

Thank you.

The include guards for all the relocated header files and the new files eg: 25 #ifndef SHAREVMRUNTIMEJVMFLAGHPP need updating now you added the extra directory.

Good catch!

Bikeshed: I'm not sure about calling the directory jvmFlags. First everything under src/hotspot is "jvm" so it is somewhat redundant to state that. Second you end up with jvmFlag/jvmFlag*.* which is more redundancy. I think runtime/flags would suffice.

Since we’re going through all the trouble, we can make this right - I like the suggestion, so will do.

cheers

Thanks, David On 31/03/2018 3:27 AM, Gerard Ziemski wrote: Hi all, Please review this large and tedious (sorry), but simple fix that accomplishes the following: #1 factor out the command option flag related APIs out of globals.hpp/.cpp into its own dedicated files, i.e. jvmFlag.hpp/.cpp #2 moved all jvmFlag* files into its own dedicated folder (i.e. src/hotspot/share/runtime/jvmFlag/) #3 merge Flag (too generic name) and CommandLineFlag classes and rename them as JVMFlag #4 cleanup globals.hpp includes originally added by the JEP-245 Note: the renamed file retain their history, but one needs to add “follow” flag, ex. “hg log -f file” https://bugs.openjdk.java.net/browse/JDK-8081519 http://cr.openjdk.java.net/~gziemski/8081519rev2 Passes Mach5 hstier1-tier5, jtreg/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges tests. cheers

More information about the hotspot-dev mailing list