jcmd - executing multiple commands at the same safepoint?

[Thomas Stüfe]

 Hi Frederic,

<snip>
>>
>> I am not saying that is what we should do, I am just saying it is not
>> difficult to implement.
>
> You’re right, this implementation is not as complex as I thought.
> However, the way this code works silently add a new constraint to
> diagnostic commands: a diagnostic command cannot take in
> argument the name of another diagnostic command anymore.
>

I am starting to dislike this solution too, but I think that
constraint alone would not be a problem.

>>
>> Note that the patch does not touch the Safepoint issue, apart from
>> introducing DCmd::can_run_at_safepoint() which could be implemented by
>> each command similar to the other virtual functions and could be used
>> in parse_and_execute to decide command bundling.
>>
>>>
>>>> This should work for JFR too, if your parameters do not contain
>>>> whitespace-enclosed strings which equal valid command names:
>>>>
>>>> JFR.start threads={Thread1,Thread2, .*Pool.*, VM.info}
>>>>
>>>> would be invalid since "VM.info" is a valid command name.
>>>>
>>>> If that proposal does not work, an explicit notation may be needed,
>>>> e.g. the initially mentioned enclosing-by-brackets:
>>>>
>>>> jcmd { VM.version, VM.metaspace basic, VM.uptime}
>>>>
>>>> ----
>>>>
>>>> 2) Yes, there are commands which cannot be run at a safepoint. I would
>>>> make that a property of the DiagnosticCmd object, and just forbid to
>>>> stack them with other commands.
>>>
>>>
>>> This is another big issue. The DCmd framework imposes very little
>>> limitation to what command can go. Any command invoking Java code,
>>> like ManagementAgent.start, would have troubles if executed at a
>>> safepoint.
>>
>> As sketched out, I would envison a DCmd::can_run_at_safepoint()
>> virtual function, where each command can decide if it 1) needs a
>> safepoint, 2) does not care or 3) cannot run in a safepoint. Actually,
>> that way we could even centralize safepoint handling out of the
>> individual commands into the central loop, at least for all commands
>> which need safepoints.
>
> Many diagnostic commands re-use some existing code that can be
> called from other APIs, which means we cannot always move the
> safepoint request outside of the diagnostic command.

I may have been unclear. What I meant was: there is a repeated pattern
where directly in xxxDCmd::execute() a VMOp is prepped and fired. I
was vaguely thinking that if we give the DCmd a "needs-safepoint"
attribute, whoever fires the command could then also create the
safepoint and fire it.

You are right, untouched from that idea are the cases where the DCmd
calls a shared utility function which is called from other places and
which creates a safepoint, e.g. HeapDumpDCmd::execute() /
HeapDumper::dump().

Instead, here we will have to deal with safepoint nesting if this
command is executed together with other commands inside an outer
safepoint, but that should not be a problem, or?

<snip>

>>
>> One argument in favor of letting the user decide is that running
>> multiple lengthy commands at a safepoint may trigger the safepoint
>> timeout. So, this could be a reason why one would want to avoid
>> automatic-command-bundling. I don't know... that is bikeshed
>> territory.
>
> I think this is an important point. We cannot simply decide that when
> multiple commands are passed to jcmd, they will always be executed
> under a single safepoint because:
>   1 - this would break existing scripts using multiple commands with
>        at least of them not supporting execution at a safepoint
>   2 - safepoint timeouts will be much more likely, and may not even
>        occur before deployment
>

Yes, I am convinced now. Caller should have to explicitly specify that
his command bundle is to be executed at a safepoint.

>>
>>> Today, commands are not necessarily executed at a safepoint. Each
>>> command has to request a safepoint if it needs it.
>>>
>>> A way this could work would be to have:
>>>  - a special command to specify that all following commands in the
>>>    command string must be executed within the same safepoint
>>>  - this special safepoint will check that the following commands
>>>    are safe to be executed at a safepoint (this must be a new
>>>    property of each command as state by Thomas). If any command
>>>    is not safepoint-safe, the whole command string would be
>>>    rejected
>>>  - then this special command would trigger a safepoint, and then
>>>    invoke the following commands sequentially
>>
>> I think whether to implement this as another command or to bake this
>> into the parser some other way is an implementation detail. I would
>> rather start out defining the syntax, which has to be backward
>> compatible (so, the response of a new VM with the new stacking
>> capability to an old jcmd talking to it must remain unchanged). This
>> needs a CSR and probably a long discussion. If my past experiences are
>> a guide, this will take time.
>
> I think that it is a fundamental piece of the design. Adding a new diagnostic
> command to group several commands under a unique safepoint:
>   1 - requires explicit action from the user (and acknowledgment of the risks)
>   2 - doesn’t touch the current syntax/semantic at all
>
> Another reason I’m pushing for a specific diagnostic command rather than
> changing the syntax is remote execution. jcmd is not the only way to invoke
> a diagnostic command, they can also be invoke remotely using JMX and the
> com.sun.management.DiagnosticCommand dynamic MBean. Through the
> platform MBean, users do not have access to the jcmd syntax, they see
> each diagnostic command as a MBean operation (providing that they have
> been registered with the DCmd_Source_MBean flag). The initial support
> for remote execution was taking a single String and processed it just like
> a string passed by jcmd, but people developing consoles for remote
> management complained that this was not convenient for them, and asked
> for something they could use more programmatically.
> If the safepoint bundling is implemented in the syntax, JMX clients won’t
> be able to use the feature. If we implement it with a new diagnostic command,
> it will be visible from the MBean and they will be able to use it.

That is interesting. Do you have examples for such programs? I was
looking for ways of how to use jcmd (or equivalent) remotely.

>
> At last, creating a specific diagnostic command for safepoint bundling would
> show the way on how to group commands for consistency. Safepoints is not
> the only way to get consistency. For some JVM internals reasons, you might
> want to group commands under a single lock, like the Threads_lock. Adding
> a diagnostic command for each type of bundling seems more extensible.
>

You make good points.

Adding a new command has also the advantage that it does not affect
downward compatibility and hence would not require a CSR. CSR seems to
slow down things a bit (I have a very simple CSR for jcmd open
undecided since 12 days:
https://bugs.openjdk.java.net/browse/JDK-8203043). So, while I think
CSR are a necessary process, I am happy to avoid them if possible.

Lets spin this out a bit more:

-> However the user specifies "please execute these commands at a
single safepoint", if one of the enclosed commands cannot be run
inside a safepoint the whole bundle should fail, agreed? So,
specifying e.g. JFR.start as part of such a command bundle would cause
the whole bundle to not being executed.

-> If we introduce a command like you propose (lets call it "VM.run"
for the sake of the argument) we still need to decide how to delimit
the commands at the command line. That brings us back to the syntax
question: separation by a explicit delimiter, if possible one that
does not clash with common unix shells?

Lets try colon ':' :

jcmd VM.run VM.uptime : VM.metaspace show-loaders show-classes
by-chunktype : VM.classloaders show-classes : VM.info

Of course, we can wrap all arguments to jcmd in quotes, then the
delimiter sign does not affect things as much, unless we use quotes :)
We could e.g. use semicolon:

jcmd ' VM.run VM.uptime ; VM.metaspace show-loaders show-classes
by-chunktype ; VM.classloaders show-classes ; VM.info '

Or, we go back to my original idea of letting the command keyword be
the indicator for a new command start. Honestly, at this point I have
no preference. Both approaches seem reasonable.

The only niggle I have with an explicit command is the syntax, which I
find a bit awkward. My thinking before reading your response was that
I'd prefer a jcmd flag to specify the same behavior: that all commands
should be executed at a single safepoint. E.g. [-s/--stacked]

jcmd -s ' VM.uptime ; VM.metaspace show-loaders show-classes
by-chunktype ; VM.classloaders show-classes ; VM.info '

But I see the disadvantage: depending on how it is implemented this
would require consistent changes in both jcmd and hotspot, and not
work with other clients using MBeans. Whereas the hypothetical
"VM.run" would work out of the box for any old jcmd version and any
other client.

Of course one could keep the "VM.run" idea and still give jcmd a
"-s/--stacked" option, as a shallow convenience feature, which just
translates to "VM.run" and runs that.

-> A variation of your "have special commands to run a command bundle
in one particular context" theme could be having only one command,
e.g. "VM.run", specifying the conditions under which to run the bundle
as sub options. E.g. "VM.run -s <commands>"  for "run commands at
safepoint", "VM.run -t <commands>" for "run commands under
Threads_lock". That would even allow to combine these contexts. I do
not know if that makes sense.

- Another advantage of having an explicit command just occurring to me
is that it enables more evolved command files, where one can
intersperse command bundles and single commands. For instance, I could
give this to a customer:

<command file>
1  VM.uptime
2  VM.run_commands VM.metaspace show-loaders show-classes by-chunktype
VM.classloaders details VM.classloader_stats
3  GC.run
4  VM.run_commands VM.metaspace show-loaders show-classes by-chunktype
VM.classloaders details VM.classloader_stats

to execute a command bundle before and after triggering a GC.

> We have a three days week-end in the US, but I should be able to code
> something next week.
>

Oh, you want to take over the implementation, sure! Fine by me.

> Thank you for your ideas and your interest in diagnostic commands.

Sure. Why I am interested in this: We strongly rely on tools similar
to jcmd for our support. We have quite capable tools in our (licensed,
non-open) VM port, which are similar to jcmd in many aspects. So we
try to bring some of the capabilities upstream, in order to ease the
merge costs for us and to help the OpenJDK community.

Have a nice weekend,

Best Regards, Thomas

>
> Fred
>
>
>>
>>>
>>> A new separator, more convenient than newline, would be require to have
>>> a single line syntax.
>>>
>>> My 2 cents,
>>>
>>
>> Certainly worth more than 2 cents :) Thanks for your thoughts!
>>
>> ..Thomas
>>
>>> Fred
>>>
>>>
>>>>
>>>>>
>>>>> Erik
>>>>>
>>>>> Hi Thomas,
>>>>>
>>>>> very positive suggestion.
>>>>>
>>>>> One observation:
>>>>> There already seems to be some different interpretation of the command
>>>>> line
>>>>> in different Java versions:
>>>>> For instance when I try to dump a flight recording in 1.8.0_152-ea-b05 :
>>>>> I
>>>>> can use:
>>>>> jcmd 33014 JFR.dump filename="recording1.jfr" recording=1
>>>>> but in build 9+181 , the same command must be:
>>>>> jcmd 33014 JFR.dump filename="recording1.jfr",recording=1
>>>>> (the comma to separate sub-options you mentioned earlier)
>>>>>
>>>>> Therefore the suggestion without curly brackets, giving several commands
>>>>> after one another seems good with regard to backwards compatibility.
>>>>>
>>>>> Motivation: hawt.io uses the MBean
>>>>> com.sun.management:type=DiagnosticCommand
>>>>> to access the same functionality as jcmd. Variations in option syntax
>>>>> across
>>>>> Java versions is already an issue (only affecting sub-options though, as
>>>>> each command is a separate JMX operation). So syntax compatibility is
>>>>> highly
>>>>> appreciated :-)
>>>>>
>>>>> Martin
>>>>>
>>>>> lør. 12. mai 2018 kl. 20:11 skrev Kirk Pepperdine
>>>>> <kirk.pepperdine at gmail.com>:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On May 10, 2018, at 11:26 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> On Thu, May 10, 2018 at 9:13 AM, Kirk Pepperdine
>>>>>>> <kirk.pepperdine at gmail.com> wrote:
>>>>>>>>
>>>>>>>> The stacking at the safe point would be a huge win. Right now thread
>>>>>>>> dump consistency can really confuse people as the tooling will show
>>>>>>>> two
>>>>>>>> threads owning the same lock at seemingly the same time. Reality, it’s
>>>>>>>> just
>>>>>>>> that people didn’t realize you get a safe point for each thread
>>>>>>>> therefor
>>>>>>>> there is motion in the system as you’re collecting the data.
>>>>>>>
>>>>>>>
>>>>>>> I am a bit confused. What tooling are you talking about?
>>>>>>
>>>>>>
>>>>>> jstack. I’ve not seen it with jcmd. But I often see 2 threads holding
>>>>>> the
>>>>>> same lock at the “same time” which is of course non-sense. I can dig one
>>>>>> up
>>>>>> for you if you’re still confused.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> As an aside, it’s amazing how many dev’s aren’t aware of jcmd. Just
>>>>>>>> yesterday after my session at Devoxx I had someone ask me about using
>>>>>>>> jfr
>>>>>>>> from the command line, many in that session had not seen jcmd before.
>>>>>>>> The
>>>>>>>> feedback was, wow, this is very useful and I wished I had of known
>>>>>>>> about it
>>>>>>>> earlier.
>>>>>>>
>>>>>>>
>>>>>>> Yes, jcmd is quite useful. I also really like the simple design, which
>>>>>>> is by default up- and downward compatible (so you can talk to any VM,
>>>>>>> new, old, it should not matter). That is just good design. We - Sap -
>>>>>>> work to extend jcmd, to help our support folks. E.g. the whole new
>>>>>>> VM.metaspace command chain.
>>>>>>
>>>>>>
>>>>>> And simple it is….well done!!!
>>>>>>
>>>>>> — Kirk
>>>>>>
>>>>>
>>>
>