[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(S): 8186902: jcmd GC.run should not be blocked by DisableExplicitGC
From:       Kevin Walls <kevin.walls () oracle ! com>
Date:       2017-09-01 8:01:39
Message-ID: 10f976ad-ad94-5585-6003-22057fb393e9 () oracle ! com
[Download RAW message or body]


Thanks Serguei, and Mikael!



On 30/08/2017 00:33, serguei.spitsyn@oracle.com wrote:
> Hi Kevin,
>
> It looks good to me.
>
> Thanks,
> Serguei
>
>
> On 8/29/17 04:41, Kevin Walls wrote:
>> Hi,
>>
>> This is a small review request for:
>>
>> 8186902: jcmd GC.run should not be blocked by DisableExplicitGC
>> https://bugs.openjdk.java.net/browse/JDK-8186902
>>
>> jcmd GC.run to invoke GC fails if -XX:+DisableExplicitGC is set: this 
>> seems like a mistake as it is obstructive for a live app that needs a 
>> GC, and was started with -XX:+DisableExplicitGC.
>>
>> hg diff pasted below, simply removes the DisableExplicitGC if and 
>> un-indents the existing call to collect().
>>
>> Builds and manually tests OK, GC occurs in response to jcmd GC.run 
>> even if java was started with -XX:+DisableExplicitGC
>>
>> Previous email on this: 
>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-August/021748.html
>>
>> Many thanks
>> Kevin
>>
>>
>> bash-4.2$ cd hotspot
>> bash-4.2$ hg diff
>> diff -r a20f0fa4c426 src/share/vm/services/diagnosticCommand.cpp
>> --- a/src/share/vm/services/diagnosticCommand.cpp             Mon Aug 28 
>> 23:46:22 2017 +0000
>> +++ b/src/share/vm/services/diagnosticCommand.cpp             Tue Aug 29 
>> 02:55:56 2017 -0700
>> @@ -414,11 +414,7 @@
>>   }
>>
>>   void SystemGCDCmd::execute(DCmdSource source, TRAPS) {
>> -   if (!DisableExplicitGC) {
>> -       Universe::heap()->collect(GCCause::_dcmd_gc_run);
>> -   } else {
>> -       output()->print_cr("Explicit GC is disabled, no GC has been 
>> performed.");
>> -   }
>> +   Universe::heap()->collect(GCCause::_dcmd_gc_run);
>>   }
>>
>>   void RunFinalizationDCmd::execute(DCmdSource source, TRAPS) {
>> bash-4.2$
>>
>>
>>
>>
>> On 29/08/2017 10:14, Kevin Walls wrote:
>>>
>>> Hi Mikael, thanks yes, it could be a separate cmd GC.runForce... 
>>> However I was thinking if you can get as far as having your jcmd 
>>> executed, you really _do_ want to run that collection.   Whatever 
>>> behaviour you were protecting against when you chose the 
>>> command-line arguments, you would only ever want to override if you 
>>> run the jcmd to invoke GC... 8-)
>>>
>>> I'll convert this to a review request for removing that check, will 
>>> post that shortly.   This would be changing the behaviour, but I 
>>> don't think it contradicts anything we document, and we seem to have 
>>> added the check without documenting it.
>>>
>>> Thanks
>>> Kevin
>>>
>>>
>>> On 28/08/2017 16:01, Mikael Gerdin wrote:
>>>> Hi Kevin,
>>>>
>>>> On 2017-08-22 16:38, Kevin Walls wrote:
>>>>> Hi,
>>>>>
>>>>> jcmd GC.run to invoke GC fails if -XX:+DisableExplicitGC is set: 
>>>>> this seems like a mistake?
>>>>>
>>>>> This behaviour is obstructive for a live app that _needs_ a GC, 
>>>>> and was started with -XX:+DisableExplicitGC.
>>>>>
>>>>> DisableExplicitGC to protect from Java code calling System.gc 
>>>>> frequently makes sense, but if I can attach and run a dcmd, I 
>>>>> should have permission to inspect and maintain the JVM, including 
>>>>> invoking a GC. (This is as the user who owns the process and can 
>>>>> kill it off.)
>>>>>
>>>>> The behaviour (checking DisableExplicitGC in 
>>>>> SystemGCDCmd::execute) comes in with:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8004095
>>>>> 8004095: Add support for JMX interface to Diagnostic Framework and 
>>>>> Commands
>>>>>
>>>>> The JMX relation I suppose suggests we didn't want JMX to override 
>>>>> DisableExplicitGC by way of using a jcmd/DCmd.
>>>>>
>>>>> But also, we now have:
>>>>> 8072913: [REDO] GCCause should distinguish jcmd GC.run from 
>>>>> System.gc()
>>>>> Summary: GCCause which is caused by GC.run diagnostic command 
>>>>> should be different from System.gc() .
>>>>>
>>>>> ..at least the causes are distinct.
>>>>>
>>>>> I don't think we document this clearly.   Our comment in 
>>>>> globals.hpp is ""Ignore calls to System.gc()".   I don't think we 
>>>>> say anywhere that jcmd is subject to being disabled by the flag.
>>>>>
>>>>> Interested to hear any reason in favour of the current behaviour!   
>>>>> If there's nothing, I'll log a bug and ask for review of the 
>>>>> change to remove it...
>>>>
>>>> There were some discussions earlier around this area and I came up 
>>>> with the idea of having a "force" option to the GC.run command to 
>>>> override DisableExplicitGC.
>>>> The comments in globals.hpp are a notoriously bad spec for the 
>>>> flags since they are only ever present in debug builds of the JVM.
>>>>
>>>> Thanks
>>>> /Mikael
>>>>
>>>>>
>>>>> Thanks
>>>>> Kevin
>>>>>
>>>>>
>>>>>
>>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]