RFR: 8243208: Clean up JVMFlag implementation [v3]

Ioi Lam iklam at openjdk.java.net
Wed Sep 9 20:53:13 UTC 2020


On Wed, 9 Sep 2020 14:12:04 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixed JVMFlag::is_writeable()
>
> src/hotspot/share/runtime/flags/jvmFlagLimit.cpp line 102:
> 
>> 100:   static constexpr const JVMFlagLimit* get_limit(const JVMTypedFlagLimit<T>* p, int dummy, ConstraintMarker
>> dummy2, short func, int phase, T min, T max) { 101:     return p;
>> 102:   }
> 
> Instead of the 'no_limit' functions, could you have get_limit have PRODUCT_RETURN(NULL) ?  Otherwise, fine to get the
> macros working.

I think your suggestion would simply the macro materialization from
#ifdef PRODUCT
  ALL_FLAGS(FLAG_LIMIT_PTR_NONE,
            FLAG_LIMIT_PTR_NONE,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR_NONE,
            APPLY_FLAG_RANGE,
            APPLY_FLAG_CONSTRAINT)
#else
  ALL_FLAGS(FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR,
            APPLY_FLAG_RANGE,
            APPLY_FLAG_CONSTRAINT)
#endif
to
ALL_FLAGS(FLAG_LIMIT_PTR_NOT_PRODUCT,
            FLAG_LIMIT_PTR_NOT_PRODUCT,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR,
            FLAG_LIMIT_PTR_NOT_PRODUCT,
            APPLY_FLAG_RANGE,
            APPLY_FLAG_CONSTRAINT)
But I would need twice the number of the get_limit functions, like:
  static constexpr const JVMFlagLimit* get_limit(const JVMTypedFlagLimit<T>* p, int dummy, T min, T max) {
    return p;    // return product flag for all builds
  }
  static constexpr const JVMFlagLimit* get_limit_not_product(const JVMTypedFlagLimit<T>* p, int dummy, T min, T max) {
    NOT_PRODUCT(return p);     // return non-product flag only in non-product builds
    PRODUCT_ONLY(return NULL);  // .... but not in product build
  }
So the code will be more verbose than before.

Instead, in [bf90a37](https://github.com/openjdk/jdk/pull/82/commits/bf90a3765720267a8770f7bb0cc6957471ccb21b), I
changed no_limit to a varargs function so now there's only one version of it.

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

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


More information about the shenandoah-dev mailing list