RFR: 8081833: Clean up JVMFlag getter/setter code

[Ioi Lam]

On Tue, 15 Sep 2020 20:34:48 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> This is a follow to [JDK-8243208 Clean up JVMFlag implementation](https://bugs.openjdk.java.net/browse/JDK-8243208).
>> 
>> I try to use templates and inheritance (and a bit of macros) to get rid of the large amount of duplicated code in the
>> JVMFlag getters/setters.
>> Stefan Karlsson and I have tried this several times already, but I think the current attempt probably has the least
>> amount of hacks and most functionality:
>> - Type safety is enforced up to the point of loading/storing `JVMFlag::_addr`, without any unchecked typecasts.
>> - Runtime type checking is implemented by the `JVM_FLAG_TYPE(t)` macro (see jvmFlagAccess.hpp), by passing along an
>>   integer (`JVMFlag::TYPE_int`, etc). This is more efficient than previous attempts that passed along a function pointer
>>   (see http://cr.openjdk.java.net/~stefank/8081833/webrev.04/)
>> - The old `JVMFlag::xxxAtPut` functions were undocumented. There are actually two groups that have different usage. I
>>   added documentation in jvmFlagAccess.hpp to explain this.
>> - I got rid of the `JVMFlagEx` class and moved the code into `JVMFlag`. This is possible with C++11 which allows forward
>>   declaration of `enum JVMFlagsEnum : int;`
>> - I changed `JVMFlag::_type` from a string into an enum. I am surprised this had lasted for over 20 years.
>> - I also changed `JVMFlag` from a struct into a class and made some fields private. I probably will do more cleanup in a
>>   separate RFE.
>> 
>> Please start with jvmFlagAccess.hpp, jvmFlagAccess.hpp and then globals_extension.hpp.
>> 
>> Probably not everyone is in love with the `JVM_FLAG_TYPE(t)` macro. I'll be happy to try out alternatives.
>
> src/hotspot/share/runtime/flags/jvmFlag.hpp line 128:
> 
>> 126:     TYPE_ccstrlist,
>> 127:     NUM_FLAG_TYPES
>> 128:   };
> 
> Are those macros really needed here and better than simply saying:
> 
>   enum FlagType : int {
>     TYPE_bool,
>     TYPE_int,
>     TYPE_uint,
>     TYPE_intx,
>     TYPE_uintx,
>     TYPE_uint64_t,
>     TYPE_size_t,
>     TYPE_double,
>     TYPE_ccstr,
>     TYPE_ccstrlist,
>     NUM_FLAG_TYPES
>   };

`JVM_FLAG_NON_STRING_TYPES_DO` is used in several places, such as declaring functions like `is_int()`, `get_int()`,
`set_int()`, etc. I think it's less tedious/error prone than writing the same code 9 times.

> src/hotspot/share/runtime/flags/jvmFlagAccess.hpp line 78:
> 
>> 76:       if (flag->type() != type_enum) {
>> 77:         return JVMFlag::WRONG_FORMAT;
>> 78:       }
> 
> Can this normally happen at runtime? Maybe replace it with assert_type() instead?

To be honest I am not sure. It's the same behavior as the olde code.

-------------

PR: https://git.openjdk.java.net/jdk/pull/163