Hi Thomas,

> On May 24, 2018, at 16:47, Thomas Stüfe <thomas.stu...@gmail.com> wrote:
> 
> Hi Frederic,
> 
> On Thu, May 24, 2018 at 7:29 PM, Frederic Parain
> <frederic.par...@oracle.com> wrote:
>> Hi Thomas,
>> 
>> 
>> On 05/24/2018 12:08 PM, Thomas Stüfe wrote:
>>> 
>>> Hi Erik,
>>> 
>>> On Tue, May 22, 2018 at 11:08 PM, Erik Gahlin <erik.gah...@oracle.com>
>>> wrote:
>>>> 
>>>> Not sure if this will have an impact, but we have plans to add
>>>> parameterization to JFR.start.
>>>> 
>>>> It will work like the recording wizard in JMC. All the the widgets you
>>>> see
>>>> in the dialog will be available from command line. For instance, you will
>>>> be
>>>> able to do something like this (syntax and naming have not been decided)
>>>> 
>>>> jcmd <pid> JFR.start gc-level=detailed exceptions=true
>>>> method-sampling=10ms
>>>> classloading=true
>>>> 
>>>> The parameters are not fixed at compile time, but instead depends on the
>>>> .jfc file in use. This will simplify usage of JFR as you will no longer
>>>> need
>>>> to export a .jfc file from JMC if you want to configure events. The
>>>> parameterization will also be available from -XX:StartFlightRecording,
>>>> which
>>>> uses a comma-separated list.
>>>> 
>>>> Furthermore, JFR also supports custom settings, which can have arbitrary
>>>> syntax and semantics. For instance, it will be possible to add a setting
>>>> that will filter out events for a particular thread and have it exposed
>>>> to
>>>> jcmd as a parameter, i.e.
>>>> 
>>>> jcmd <pid> JFR.start threads={Thread1,Thread2, .*Pool.*}
>>>> 
>>>> The diagnostic commands for JFR are implemented in Java, so they will not
>>>> be
>>>> able to execute during a safepoint, so I don't expect there to be an
>>>> issue,
>>>> but I thought I should mention it.
>>> 
>>> 
>>> Yes, it is good to know that.
>>> 
>>> There are two different problems here:
>>> 1) how to specify multiple jcmd commands in a convenient way on the
>>> command line
>>> 2) how to run them in a single safepoint.
>>> 
>>> 
>>> For (1) it turns out it already works, in a fashion:
>>> 
>>> We can specify multiple commands when they are separated by newlines.
>>> I think that is an unintended side effect of the command file support
>>> (-f), but surprisingly also works on the command line if you surround
>>> all arguments with a single quote:
>>> 
>>> thomas@mainframe /shared/projects/openjdk/jdk-jdk/output-fastdebug $
>>> ./images/jdk/bin/jcmd test3 'VM.version
>>> VM.metaspace basic
>>> VM.uptime'
>> 
>> 
>> As the author of this code, I confirm that it is an unintended side effect.
>> 
>>> which is of course completely unix-unlike and inconvenient to use. It
>>> just works because jcmd runner appends all program arguments to a
>>> single string, which it sends over to the VM.
>>> 
>>> --
>>> 
>>> My proposal would be that we change that in a backward-compatible way
>>> by specifying that
>>> - multiple commands are allowed without having to be separated by
>>> newlines. Any whitespace should be a separator:
>>> <command>[ <parameter> <parameter>] <command>[ <parameter> <parameter>]
>>> - a command name indicates start of a new command. Any token which is
>>> not a command name is a parameter for the preceeding command.
>> 
>> 
>> This syntax, without special separator, would be hard to implement.
>> The command line string is split into individual commands without
>> any knowledge about the particular syntax of each command (see
>> DCmd::parse_and_execute()). Considering all command name as a
>> separator would significantly complexify the initial parsing.
> 
> No, its not that hard, one just has to modify the parsing a bit.
> 
> But talk is cheap :)
> 
> so here is a working prototype:
> 
> http://cr.openjdk.java.net/~stuefe/webrevs/jcmd-stack-commands/webrev.00/webrev/
> 
> which modifies parsing in such a way that instead of newlines the
> commands are split at the start of the next command. Using that, I can
> specify multiple commands, including commands with parameters, at the
> command line, and they will processed sequentially:
> 
> ./images/jdk/bin/jcmd test3 VM.uptime VM.metaspace basic
> VM.classloaders verbose VM.uptime
> 19857:
> 25,108 s
> 
> Usage:
>  Non-class:      4,47 MB capacity,     4,43 MB (>99%) used,    41,36
> KB ( <1%) free+waste,     1,56 KB ( <1%) overhead.
>      Class:    392,00 KB capacity,   377,27 KB ( 96%) used,    14,23
> KB (  4%) free+waste,   512 bytes ( <1%) overhead.
>       Both:      4,86 MB capacity,     4,80 MB ( 99%) used,    55,59
> KB (  1%) free+waste,     2,06 KB ( <1%) overhead.
> 
> Virtual space:
>  Non-class space:        8,00 MB reserved,       4,75 MB ( 59%) committed
>      Class space:        1,00 GB reserved,     512,00 KB ( <1%) committed
>             Both:        1,01 GB reserved,       5,25 MB ( <1%) committed
> 
> Chunk freelists:
>   Non-Class:  4,25 KB
>       Class:  0 bytes
>        Both:  4,25 KB
> 
> +-- <bootstrap>
>      |
>      |                       CLD: 0x00007f75a41d85d0
>      |              Loader Klass: 0x0000000000000000
>      |                Loader Oop: 0x0000000000000000
>      |
>      +-- platform (instance of
> jdk.internal.loader.ClassLoaders$PlatformClassLoader)
>            |
>            |                       CLD: 0x00007f75a439b9c0
>            |              Loader Klass: 0x00000008000107b0
>            |                Loader Oop: 0x0000000715d4cb20
>            |
>            +-- app (instance of
> jdk.internal.loader.ClassLoaders$AppClassLoader)
> 
>                                          CLD: 0x00007f75a438f8e0
>                                 Loader Klass: 0x0000000800010348
>                                   Loader Oop: 0x0000000715d4efe0
> 
> 25,108 s
> 
> (Note: VM.classloaders command is not yet upstream, I was searching
> for a command with parameters and short output)
> 
> 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.

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

> 
>> 
>>> This is all still very much in the thinking phase. The more I think
>>> about this the clearer it gets that implementing it is the smallest
>>> part - more time consuming will be to file a CSR and agree on a syntax
>>> which is both backward compatible and practical on the unix command
>>> line.
>> 
>> 
>> Overall, I like the idea of grouping commands inside a single safepoint
>> to provide consistency across their results. However, I think this
>> behavior should be explicitly requested on the command line.
> 
> Yes, I get the feeling too that it may be better to let the user
> explicitly bundle commands for one-safepoint-execution. Would that
> make jcmd too difficult to use? Who knows about safepoints?
> 
> 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

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

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.

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

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

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

Reply via email to