RFR(L) 8243208 Clean up JVMFlag implementation

[Stefan Karlsson]

Hi Ioi,

I did an initial scan over the change, to see what it looked like from 
the GC perspective, but didn't actually look at the code in detail.

Would you mind making these whitespace changes to make the code a bit 
more clear and consistent:
https://cr.openjdk.java.net/~stefank/8243208/review/webrev.01/

Regarding the rename Flags to Attrs. I think this is one of those things 
that would be nice to get separated out into a preparation patch. Also, 
I'm not sure about the name Attrs. It's obscure, and even our guidelines 
discourage introducing abbreviated names. Could we rename this to 
Attributes?

I think this shouldn't be pushed. It's fine during the review, but I 
don't think we should leave these breadcrumbs in the code. It should be 
enough that it has been logged as an RFE.

// TODO:
// JVMFlagConstraintChecker is just temporary code to bridge 
JVMFlagLimit with the existing
// JVMFlagConstraint_XXX classes. All of these will be gone when we 
templatize the
// range/constraint checks (JDK-8081833).

Thanks,
StefanK

On 2020-09-02 03:29, 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