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

[Mikael Gerdin]

Hi Kevin,

On 2017-08-29 13: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$

Looks fine.
/Mikael

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