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

Kim Barrett kbarrett at openjdk.java.net
Mon Mar 1 18:13:51 UTC 2021


On Tue, 23 Feb 2021 17:31:08 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;
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   use macros to simplify assert_compatible_type; restore code in parse_xss so the comments make sense

Changes requested by kbarrett (Reviewer).

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

> 73: #ifndef PRODUCT
> 74:   int   _type_enum;
> 75: #endif

I think all the `#ifndef PRODUCT` stuff around _type_enum should instead be `#ifdef ASSERT`.  These belong in debug builds, but not in optimize builds.

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

> 293: 
> 294:   // type checking
> 295: #define CHECK_COMPATIBLE(type) \

CHECK_COMPATIBLE doesn't appear to be used outside the following assertion function.  Add an `#undef` afterward.

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

> 297:     assert(sizeof(T) == sizeof(type) && \
> 298:            std::is_integral<T>::value == std::is_integral<type>::value && \
> 299:            std::is_signed  <T>::value == std::is_signed  <type>::value, "must be"); break;

I'd prefer the `break` be on it's own source line, rather than after the assert form.

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

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


More information about the hotspot-runtime-dev mailing list