RFR: 8183569: Remove duplicated flag limits in parse_xss and globals.hpp

[Thomas Stuefe]

On Tue, 23 Feb 2021 07:55:38 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> We have the following in Arguments::parse_xss():
> 
>   const julong min_ThreadStackSize = 0;
>   const julong max_ThreadStackSize = 1 * M;
>   const julong min_size = min_ThreadStackSize * K;
>   const julong max_size = max_ThreadStackSize * K;
> 
> which duplicates the min/max range specified in globals.hpp:
> 
>   product_pd(intx, ThreadStackSize, \
>           "Thread Stack Size (in Kbytes)") \
>           range(0, 1 * M) \
> 
> I added an API to query the min/max range of a flag, so parse_xss can be rewritten as
> 
>   const JVMTypedFlagLimit<intx>* limit = 
>     JVMFlagLimit::get_range_at(FLAG_MEMBER_ENUM(ThreadStackSize))->cast<intx>();
>   const julong min_size = limit->min() * K;
>   const julong max_size = limit->max() * K;

Hi Ioi,

Seems reasonable. Some remarks below.

..Thomas

src/hotspot/share/runtime/flags/jvmFlag.hpp line 314:

> 312: #endif
> 313:   }
> 314: 

Do we need this? Seems an unused duplicate to the version in jvmFlagLimit.hpp.

src/hotspot/share/runtime/flags/jvmFlagLimit.hpp line 155:

> 153:       case JVMFlag::TYPE_uintx:    assert(sizeof(T) == sizeof(uintx)    && std::is_signed<T>::value == std::is_signed<uintx>   ::value, "must be"); break;
> 154:       case JVMFlag::TYPE_uint64_t: assert(sizeof(T) == sizeof(uint64_t) && std::is_signed<T>::value == std::is_signed<uint64_t>::value, "must be"); break;
> 155:       case JVMFlag::TYPE_size_t:   assert(sizeof(T) == sizeof(size_t)   && std::is_signed<T>::value == std::is_signed<size_t>  ::value, "must be"); break;

Could we shorten the signed comparisons to the expected default? For most types the signedness wont be a surprise :) size_t is unsigned on all our platforms. 

is_signed<bool> seems weird, isn't that UB?

src/hotspot/share/runtime/flags/jvmFlagLimit.hpp line 159:

> 157:       }
> 158:     } else {
> 159:       assert(flag_enum == JVMFlag::TYPE_double, "must be double");

Could the same comparisons not be made too here?

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

Changes requested by stuefe (Reviewer).

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