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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Fri Apr 6 20:51:58 UTC 2018


Gerard,

It looks like these files get the include jvmFlag.hpp transitively from 
globals_ext.hpp, but can you add an explicit #include 
"runtime/flags/jvmFlag.hpp" to the files that you've changed that don't 
explicitly include jvmFlagsConstraintList.hpp and others?  The include 
guards will prevent them from being included twice and it would be good 
if we ever change these to not include globals_ext.hpp.  eg:

http://cr.openjdk.java.net/~gziemski/8081519_rev2/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp.udiff.html

For the others, it's more obvious that jvmFlags.hpp will be included 
through jvmFlagConstraintList.hpp and the other jvmFlag* headers.

I still really like this cleanup!   Besides this minor comment (changes 
to several files you've already changed), it looks good!

Thanks,
Coleen

On 4/5/18 1:46 PM, Gerard Ziemski wrote:
> Hi all,
>
> Here is webrev 2 with the following additions:
>
> #1 Factor FlagSetting class out of "runtime/flags/jvmFlag.hpp" and into its own "runtime/flags/flagSetting.hpp", so that FlagSetting class can extend from “public StackObj” (thanks Coleen)
>
> #2 change the hierarchy from “runtime/jvmFlag/“ to "runtime/flags/“ (thanks David)
>
> #3 Fix the header files, so that both closed and open repository build with and without precompiled headers
>
> #4 Add “runtime/flags/*.hpp” headers to “precompiled.hpp”
>
> The number of touched files got even longer - sorry about that and big thank you for taking your time to review!
>
> Running final Mach5 hs_tier1-5 tests… (it passed earlier iterations). Passes local builds (on Mac OS X) with and without precompiled headers for both closed and open repos and both debug and product versions (should I try any other build varations?)
>
> http://cr.openjdk.java.net/~gziemski/8081519_rev2
>
>
> cheers
>
>> On Mar 29, 2018, at 2:01 PM, Gerard Ziemski <gerard.ziemski at oracle.com> 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 merge Flag (too generic name) and CommandLineFlag classes and rename them as JVMFlag
>> #3 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/8081519_rev1
>>
>> Passes Mach5 hs_tier1-tier5, jtreg/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges tests.
>>
>>
>> cheers



More information about the hotspot-dev mailing list