jcmd - executing multiple commands at the same safepoint?

[David Holmes]

Just to clarify one thing ...

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

Going to a safepoint is something that is only embodied in the actual 
VMOp. If multiple safepoint-needing VMOps are enqueued at the time of 
the safepoint then they can be coalesced and executed at the same 
safepoint. But you can't externally "go to a safepoint" then submit a 
bunch of commands and then "end safepoint". You'd need some kind of 
compound VMOp to do that.

Cheers,
David



On 26/05/2018 6:07 PM, Thomas Stüfe wrote:
>   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
>>>>>>>
>>>>>>
>>>>
>>