RFR(L) 8243208 Clean up JVMFlag implementation

[David Holmes]

Hi Ioi,

This looks really good - I like the end result in regard to the global 
definitions. I can't comment on the detailed macros or constexpr use.

Small style nit - the are few places where spaces are missing around 
operators e.g.

+  for (int i=0; i<NUM_JVMFlagsEnum; i++) {
+  assert(0 <= i && i <NUM_JVMFlagConstraintsEnum, "sanity");

Great work!

Thanks,
David
-----

On 3/09/2020 4:37 am, Ioi Lam wrote:
> I have an updated version (with fewer changes!) as suggested to me 
> Stefan Karlsson offline:
> 
> http://cr.openjdk.java.net/~iklam/jdk16/8243208-cleanup-jvmflag-impl.v02/
> 
> [1] More consistent white spaces in the macro definitions
> 
> [2] Reverted the renaming of JVMFlag::Flags. This renaming is not really 
> needed by
>      the cleanup. Doing it later will avoid unnecessary clutter in the 
> changeset.
> 
>      Also Stefan suggested renaming to JVMFlag::Attributes (instead of 
> ::Attrs).
> 
> [3] Removed some TODO comments as they are already captured in JBS issues.
> 
> Thanks
> - Ioi
> 
> 
> 
> On 9/1/20 6:29 PM, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8243208
>> http://cr.openjdk.java.net/~iklam/jdk16/8243208-cleanup-jvmflag-impl.v01/
>> https://wiki.openjdk.java.net/display/HotSpot/Hotspot+Command-line+Flags+Clean+Up+-+Design+Proposal+for+JDK-8243208 
>>
>>
>>
>>
>> Please review this major clean up of HotSpot command-line flags. The 
>> goals are:
>>
>>     + Reduce the complexity of the implementation
>>     + Improvement footprint/performance (with C++ constexpr)
>>     + Allow more flexibility in flag declaration (see also JDK-7123237)
>>
>>
>>
>> **** Notable interface changes:
>>
>>     I tried to make the changes non-intrusive. Most HotSpot developers
>>     should not be affected at all, unless you touch the following areas:
>>
>> [1] The declaration of diagnostic/experimental/manageable flags is 
>> changed.
>>     See globals.hpp for examples:
>>
>>     OLD: experimental(intx, hashCode, 5, "..docs...")
>>     NEW: product(intx, hashCode, 5, EXPERIMENTAL, "..docs...")
>>
>>     The reason is to support more flexible combinations. Before,
>>     all experimental flags must be "product". Now they can be "develop"
>>     as well.
>>
>> [2] Declaration of constraint functions is changed to use a macro.
>>     See jvmFlagConstraintsRuntime.hpp
>>
>> [3] The confusing type JVMFlag::Flags is renamed to JVMFlag::Attrs to be
>>     clear it's about "attributes of the flags" and not "the flags
>>     themselves".
>>
>> [4] The min/max of all ranges must be compile-time constants. We had
>>     two cases where the min/max was specified via os::xxx function
>>     calls. These are converted to constraint functions:
>>
>>     OLD:
>>     product_pd(uintx, InitialCodeCacheSize, "...") \
>>             range(os::vm_page_size(), max_uintx)
>>
>>     product(size_t, NUMAInterleaveGranularity, 2*M, "...")  \
>>             range(os::vm_allocation_granularity(), \
>>                   NOT_LP64(2*G) LP64_ONLY(8192*G)) \
>>
>>     NEW:
>>     product_pd(uintx, InitialCodeCacheSize, "...") \
>>             constraint(VMPageSizeConstraintFunc, AtParse)
>>
>>     product(size_t, NUMAInterleaveGranularity, 2*M, "...")  \
>>              constraint(NUMAInterleaveGranularityConstraintFunc, \
>>                         AtParse)
>>
>>
>>
>> **** Notable implementation changes:
>>
>> [1] C++ constexpr is used extensively for build-time initialization of
>>     various tables.
>>
>> [2] All XXX_FLAGS() macros now consistent take 7 arguments. Application
>>     of these macros in jvmFlag.cpp have been greatly simplified.
>>
>>     This also makes it possible for further modularization in the future
>>     (see JDK-8243205).
>>
>>     example:
>>
>>     #define RUNTIME_FLAGS(develop, \
>>                           develop_pd, \
>>                           product, \
>>                           product_pd, \
>>                           notproduct, \
>>                           range, \
>>                           constraint) \
>>
>> [3] When processing the command-line arguments, the JVM flags are now
>>     searched using a build-time generated hashtable. See 
>> jvmFlagLookup.cpp
>>
>> [4] Tables for range/constraint check used to be dynamically constructed
>>     and linearly searched. Now they are build-time generated, with O(1)
>>     access time (a simple array index).
>>
>> For detailed discussion on the new design, please see
>> https://wiki.openjdk.java.net/display/HotSpot/Hotspot+Command-line+Flags+Clean+Up+-+Design+Proposal+for+JDK-8243208 
>>
>>
>>
>>
>> **** Next steps
>>
>>     I tried to keep this changeset small. So some changes are deferred:
>>
>> [1] In an separate RFE (JDK-8081833) I will use C++ templates
>>     to consolidate the large amount of duplicated code in jvmFlag.cpp,
>>     jvmFlagRangeList.cpp and jvmFlagConstraintList.cpp.
>>
>> [2] There are archaic test cases that assume that experimental flags
>>     must be product flags. See JVMFlag::JVMFlag() constructor in 
>> jvmFlag.cpp.
>>     I'll relax this in JDK-7123237 and update the test cases.
>>
>>
>> Thanks
>> - Ioi
>