RFR(S) : 8038756 : new WB API :: get/setVMFlag

[Staffan Larsen]

On 2 apr 2014, at 13:37, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:

> Staffan,
> 
> I'd like not to have verifying of flags' value in these WB methods. Since it's a "Unsupported internal testing API", we don't have to have them.

That could work. Until someone uses the WB Api to change some flag that really should cause other flags to be updated as well. But I’ll guess we can take that problem when it comes.

> Also I'd like to have a possibility to change even "readonly" flags, which is against verifying logic I guess, e.g. UseCodeCacheFlushing for stress testing of code cache (JDK-8028595).

Changing a flag that is not meant to be changed at runtime (not marked as ‘manageable’ or ‘read_write') could cause problems. For example if having the flag set requires some init code to have been run. If the flag is safe for changing at runtime, can’t you just mark it as read_write?

/Staffan

> 
> Thanks
> Igor
> 
> On 04/02/2014 02:27 PM, Staffan Larsen wrote:
>> 
>> On 1 apr 2014, at 16:25, Lev Priima <lev.priima at oracle.com> wrote:
>> 
>>> Staffan,
>>> 
>>> sun.tools.* API(as well as all tools.jar) does not present in compact1 profile.
>> 
>> I see, that’s a reason to add this. However, do we really need to test this specifically on compact1? I’m not sure what tests you are planning to use this feature for, but if it is for testing code that we don’t believe is special for compact1, then I’m not sure we need to run the tests on compact1. Maybe a stupid optimization.
>> 
>> One larger problem here is that this introduced yet another way to set the flags. So far we have three other ways: attachListener.cpp, management.cpp, and arguments.cpp. Currently these three all have duplicated code for verifying the consistency of some of the flags. See the calls to Arguments::verify_MinHeapFreeRatio() for example. If we add yet another way, then that code needs to be copied again. I didn’t like the copying in the first place, but allowed it because we were too lazy at that point to refactor the code… At this point, however, I would like to see that duplication taken care of before we add another way to set the flags. I’m sorry that this is going to make this change harder for you, but we really should only have that code in one place.
>> 
>> /Staffan
>> 
>> 
>>> 
>>> Lev
>>> 
>>> On 04/01/2014 05:02 PM, Staffan Larsen wrote:
>>>> There is already an API for getting / setting flags in sun.tools.attach.HotSpotVirtualMachine. If at all possible, we should reuse that API instead of adding a new one.
>>>> 
>>>> Typical use:
>>>> 
>>>> HotSpotVirtualMachine vm = (HotSpotVirtualMachine) VirtualMachine.attach(pid);
>>>> vm.setFlag(“MyFlag”, “MyValue”);
>>>> 
>>>> There is also an API in sun.management.HotSpotDiagnostic for setting / getting as well as querying for options in JVM.
>>>> 
>>>> Thanks,
>>>> /Staffan
>>>> 
>>>> 
>>>> On 1 apr 2014, at 11:38, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>> 
>>>>> adding hotspot-dev alias.
>>>>> 
>>>>> On 03/31/2014 01:07 AM, Igor Ignatyev wrote:
>>>>>> Hi all,
>>>>>> 
>>>>>> Please review the patch which introduces new WhiteBox methods to get and
>>>>>> set VM flags.
>>>>>> 
>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8038756/webrev.00/
>>>>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8038756
>>>>>> testing: jprt
>>> 
>>