Hi Fred,

Thanks for this and the analysis below.

My 2c is that it is far too risky to provide a general purpose facility like this where arbitrary command combinations can result in crashing the target VM!

Certainly no command that requires a JavaThread should be allowed to be requested - only commands actually implemented via safepointing VMOps should be considered. And then we should perhaps constrain things further so that no commands that require prolog or epilog logic are accepted.

Turning things around, perhaps we should flag which Dcmds, that operate at a safepoint, are considered "combinable" and only allow those commands to be specified?

If we want a general purpose way of expressing a list of Dcmds to execute then perhaps it should be specified to process each command in turn, with a sequence of combinable safepoint ops processed at a single safepoint?

Thanks,
David

On 16/06/2018 1:32 AM, Frederic Parain wrote:
Hi,

Here’s a first prototype to explore running multiple commands in a single 
safepoint:

http://cr.openjdk.java.net/~fparain/DCmdRunAtSafepoint/webrev.00/

The syntax is very simple, with the character ‘+’ used as the delimiter between 
commands.

$ ./build/macosx-x64-debug/images/jdk/bin/jcmd 21164 VM.run_at_safepoint 
VM.version + VM.uptime
21164:
Java HotSpot(TM) 64-Bit Server VM version 
11-internal+0-2018-05-25-2202478.fparain.DCmd
JDK 11.0.0
44.143 s

Playing a bit with it, it quickly shows what are the issues when trying to run
diagnostic commands at a safepoint.

Issue 1: One important point to remember is that only the VMThread can run at a 
safepoint,
all JavaThreads are strictly forbidden to run during the safepoint. So 
executing commands at a
safepoint means they will be executed by the VMThread. Unfortunately, the 
VMThread is
subject to restrictions and cannot execute all code that a JavaThread can 
execute. This
issue is visible with the VM.info command, which takes the Heap_lock. The 
VMThread
is not allowed to grab this lock because it could cause the VMThread to be 
blocked by
a JavaThread. So far, VM.info cannot be run at a safepoint.

Issue 2: Executing a code at a safepoint requires a VMOperation. VMOperations 
are
made of a prologue, a doit() method witch contains the code executed at the 
safepoint,
and an epilogue. The prologue and the epilogue are executed by the requester of 
the
VMOperation, usually a JavaThread, and they are not executed at a safepoint. 
Prologues
and epilogues are designed to execute code the VMThread cannot execute, like 
taking
locks or loading classes. When the VMThread itself wants to execute a 
VMOperation,
what is called a nested VMOperation, it only executes the doit() method, 
skipping the
prologue and the epilogue. This is an issue for diagnostic commands using a VMOp
in their implementation: when these commands are called while the JVM is already
at a safepoint, their prologue and epilogue cannot be executed. This issue 
impacts
Thread.print and GC.run.

There are probably other issues still to be found, so I encourage people to test
this patch to see if the combinations of commands they would like to run at a
safepoint can indeed be run in this mode.

Note that the support for invoking the new command from the DiagnosticCommand
MBean has not be implemented. This is a not negligible work in order to 
correctly
check Java Permissions before executing the commands.

Let me know if you consider this code useful or if the limitations described 
above
make this feature not as pretty as we wanted it to be.

Regards,

Fred


On May 29, 2018, at 15:54, Frederic Parain <frederic.par...@oracle.com> wrote:

Hi Thomas,


On May 26, 2018, at 04:07, Thomas Stüfe <thomas.stu...@gmail.com> wrote:

<snip>

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.

If you’re looking for a console application to invoke diagnostic command
on a remote JVM, Mission Control has a dedicated support for that in the
“Diagnostic Commands” tab.
If you’re looking for exemples of source code doing remote invocations,
there’re several unit tests for the DiagnosticCommand MBean in the directory
test/jdk/com/sun/management/DiagnosticCommandMBean.



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.

Yes. The new command N would ask for the execution of a list of
commands { A, B, C, … }, and would fail if:
  - any of the specified commands { A, B, C, … } is not a registered
    command
  - any of the specified commands { A, B, C, … } doesn’t support
    execution at safepoint
  - any of the specified commands { A, B, C, … } is not exported to
    the DCmdSource used to invoke N (*)

(*) Not all commands are exported to all interfaces (jcmd,JMX), for
instance, some commands related to the JMX agent are cannot
be invoked through the JMX. Even if the new command can be
invoked from all interfaces, it must not allow to circumvent this
restriction. This should not be an issue today, because all
JMX agent commands are calling back Java code, and are not
suitable for execution at a safepoint. But this would ensure the
consistency of the model in the long term (and the test is cheap).


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


I thought about colon and semi-colon too, because they are the most
obvious delimiters. However, they are already as delimiters in classpaths
(semi-colon on Unix, colon on Windows), so parsing to find the right
delimiters would be tricky.

I’d start with ‘!’ or ‘+’ in a first time, to make some progress on the other
aspects of the command, and buy some time to discuss the delimiter
issue in parallel.

The last resort would be to force each command in the bundle to
be specified with quotes (arguments included). But it would make the
syntax very heavy, I’d prefer a simpler delimiter.


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.

This would  work fine . The parser of the jcmd string will split the
string based on ‘\n’ characters, then each line of command will be executed
sequentially, and lines containing the VM.run_command will then create the
safepoint and trigger the execution of the different commands specified in
its argument list.


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.

I’m not promising an implementation, I haven’t worked on this code
since I left the serviceability team. I need to code at least a skeleton
of the command to refresh my memory about all aspects of the
diagnostic command framework.


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.

The diagnostic command framework was designed to support an
extensible set of commands. Feel free to extend, and thank you
for contributing to the OpenJDK community.

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