RFR: 8243208: Clean up JVMFlag implementation [v3]

Ioi Lam iklam at openjdk.java.net
Wed Sep 9 15:41:40 UTC 2020


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

>> src/hotspot/share/runtime/flags/jvmFlag.cpp line 266:
>> 
>>> 264:
>>> 265: bool JVMFlag::is_writeable() const {
>>> 266:   return is_manageable();
>> 
>> Unclear why is_writeable() reduces to is_manageable(). Aren't all product flags writeable? And aren't all develop flags
>> writeable in non-product builds?
>
> All flags shouldn't be also writeable.  Interpreter code generation depends on static value of some flags, for instance.

is_writeable() doesn't mean "modifiable". The meaning is

// manageable flags are writeable external product flags.
//    They are dynamically writeable through the JDK management interface
//    (com.sun.management.HotSpotDiagnosticMXBean API) and also through JConsole.

See also
[writeableFlags.cpp](https://github.com/openjdk/jdk/blob/433394203dce55db95caefcb57bac9ec114ebc47/src/hotspot/share/services/writeableFlags.cpp#L283).

There used to be `product_rw` flags, which were "writeable" but not "manageable". However, there aren't any of them
anymore. So I have deleted product_rw() from the comments.

/**** removed comments ****/
// product_rw flags are writeable internal product flags.
//    They are like "manageable" flags but for internal/private use.
//    The list of product_rw flags are internal/private flags which
//    may be changed/removed in a future release.  It can be set
//    through the management interface to get/set value
//    when the name of flag is supplied.
//
//    A flag can be made as "product_rw" only if
//    - the VM implementation supports dynamic setting of the flag.
//      This implies that the VM must *always* query the flag variable
//      and not reuse state related to the flag state at any given time.
//
// Note that when there is a need to support develop flags to be writeable,
// it can be done in the same way as product_rw.

I also removed the implementation of product_rw(). Line 269 is part of the product_rw() removal.

I considered adding support for product_rw() in the new flags implementation. It would be fairly trivial (similar to
how MANAGEABLE is supported) but I am hesitant to do it because:

- No one uses it currently
- There are no test cases

I don't want to have an untested implementation which someone might accidently use in the future without any testing.
If anyone indeed finds producy_rw useful, they can easily implement the functionality and can write tests for it at the
same time.

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

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


More information about the shenandoah-dev mailing list