On Sun, May 27, 2018 at 12:59 AM, David Holmes <david.hol...@oracle.com> wrote:
> 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.
>
Yes, that was what I meant. I was not clear enough.
Best Regards, Thomas
> 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.pepperd...@gmail.com>:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> On May 10, 2018, at 11:26 AM, Thomas Stüfe
>>>>>>>>> <thomas.stu...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On Thu, May 10, 2018 at 9:13 AM, Kirk Pepperdine
>>>>>>>>> <kirk.pepperd...@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
>>>>>>>>
>>>>>>>
>>>>>
>>>
>
