RFR(S): 8061443: Whitebox get*VMFlag() methods fail with develop flags in product builds

[Tobias Hartmann]

Vladimir, David, Staffan,

thanks for the reviews.

On 23.10.2014 09:06, Staffan Larsen wrote:
> 
> On 23 okt 2014, at 04:35, David Holmes <david.holmes at oracle.com> wrote:
> 
>> On 23/10/2014 7:22 AM, Vladimir Kozlov wrote:
>>> getVMOption() specification does not say anything about product, locked
>>> or develop flags. "If a VM option of the given name does not exist" can
>>> interpreted different ways.
>>
>> Perhaps, but conceptually develop flags are not part of a product build and by definition non-product flags are not part of a product build, so I think the existing jmm code is quite right to exclude them and the new code should do the same. It doesn't make sense to me to present someone with a list of flags that can't actually be used.
> 
> I agree with David here. Instead of changing the JMM API, you can add functionality to the WhiteBox API to get the value of non-product flags.

I agree. I reverted the JMM changes and added a Whitebox API method
'isDebugVMFlag' to check whether a flag is debug/notproduct.

I changed the tests such that we only use JMM to verify the values returned by
the Whitebox API if the flag is accessible (i.e. is not a debug/notproduct flag
in a product build).

New webrev: http://cr.openjdk.java.net/~thartmann/8061443/webrev.01/

Thanks,
Tobias

> /Staffan
> 
>>
>> Thanks,
>> David
>>
>>> http://docs.oracle.com/javase/8/docs/jre/api/management/extension/com/sun/management/HotSpotDiagnosticMXBean.html
>>>
>>>
>>> VMOption getVMOption(String name)
>>>
>>>     Returns a VMOption object for a VM option of the given name.
>>>
>>>     Returns:
>>>         a VMOption object for a VM option of the given name.
>>>     Throws:
>>>         NullPointerException - if name is null.
>>>         IllegalArgumentException - if a VM option of the given name
>>> does not exist.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 10/22/14 6:46 AM, Tobias Hartmann wrote:
>>>> Hi David,
>>>>
>>>> On 22.10.2014 14:33, David Holmes wrote:
>>>>> On 22/10/2014 6:48 PM, Tobias Hartmann wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> On 22.10.2014 04:16, David Holmes wrote:
>>>>>>> Hi Tobias,
>>>>>>>
>>>>>>> On 21/10/2014 11:08 PM, Tobias Hartmann wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> please review the following patch.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8061443
>>>>>>>> Webrev: http://cr.openjdk.java.net/~thartmann/8061443/webrev.00/
>>>>>>>>
>>>>>>>> Problem:
>>>>>>>> The Whitebox API methods get*VMFlag() fail with develop/notproduct
>>>>>>>> flags in
>>>>>>>> product builds.
>>>>>>>>
>>>>>>>> Solution:
>>>>>>>> The code is changed to invoke 'Flag::find_flag' with
>>>>>>>> allow_locked=true and
>>>>>>>> return_flag=true to return both locked and notproduct/develop
>>>>>>>> flags. I also
>>>>>>>> changed the JVM monitoring and management code 'jmm_GetVMGlobals()'.
>>>>>>>
>>>>>>> Why did you change the jmm code? You seemed to have changed its
>>>>>>> semantics -
>>>>>>> which may require a CCC request.
>>>>>>
>>>>>> I changed the jmm code because the whitebox tests use a JMM call to
>>>>>> verify the
>>>>>> return value (see 'VmFlagTest<T>.getVMOptionAsString()').
>>>>>>
>>>>>> Do you think a CCC request is necessary here?
>>>>>
>>>>> Depends what the jmm code is supposed to return - I don't know the
>>>>> spec. But
>>>>> changing the behaviour for the sake of a test without consideration
>>>>> of real
>>>>> users is not something that should be done lightly.
>>>>
>>>> I agree. I filed a CCC request and attached the link to the bug.
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>>>
>>>>> David
>>>>>
>>>>>> Thanks,
>>>>>> Tobias
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> I adapted the existing tests to check for develop flags as well. I
>>>>>>>> noticed that
>>>>>>>> HotspotDiagnosticMXBean.getVMOption() fails with double flags and
>>>>>>>> filed
>>>>>>>> JDK-8061616 [1].
>>>>>>>>
>>>>>>>> Testing:
>>>>>>>> - New testcases
>>>>>>>> - JPRT
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Tobias
>>>>>>>>
>>>>>>>>
>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8061616
>>>>>>>>
>