RFR(S): 8186902: jcmd GC.run should not be blocked by DisableExplicitGC

[Kevin Walls]

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
>>>
>>>
>>>
>