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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Fri Mar 30 17:50:40 UTC 2018



On 3/30/18 1:47 PM, Gerard Ziemski wrote:
> Thank you Vladimir for the review.
>
>> On Mar 30, 2018, at 12:33 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>> Renaming and other changes looks good to me.
>>
>> But my suggestion about new directory was to have it on the same level as runtime/ directory and not inside it.
> I thought about that, and decided that "share/runtime/jvmFlag" better describes the purpose of the files there than just "share/jvmFlag".
>
> We have the precedence of others that extend the hierarchy in similar matter like “share/gc/g1, share/gc/parallel”, though “share/gc/“ itself has no files on its own.
>
> So, can we leave thing as proposed or do we need further discussion?

I like the sub runtime level for these flags.   I'll review the rest later.
Coleen
>
>
> cheers
>
>> I also thought you will move globals* files and test_globals.cpp but I am fine with leaving them as they are.
>>
>> Thanks,
>> Vladimir
>>
>> On 3/30/18 10: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/8081519_rev2
>>> Passes Mach5 hs_tier1-tier5, jtreg/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges tests.
>>> cheers



More information about the hotspot-dev mailing list