Request for Review (XXL): 7104647: Adding a diagnostic command framework

[Frederic Parain]

Hi Robert,

These issues should be solved with the new webrevs:

http://cr.openjdk.java.net/~fparain/7104647/webrev.hotspot.03/
http://cr.openjdk.java.net/~fparain/7104647/webrev.jdk.03/

Regards,

Fred

On 12/ 9/11 03:17 PM, Robert Ottenhag wrote:
> Adding to the review of jmm.h, that is modified in both the jdk part
> and the hotspot part, these files should be identical, without any
> differences in whitespace.
>
> Also, regarding whitespace and indentation, the use TAB characters in
> many files in the jdk patch makes jcheck complain (when trying to
> import your patch locally), and should be replaced by spaces.
>
> Best regards,
>
> /Robert
>
>
>> -----Original Message----- From: Paul Hohensee Sent: Thursday,
>> December 08, 2011 7:26 PM To: Frederic Parain Cc:
>> serviceability-dev at openjdk.java.net;
>> core-libs-dev at openjdk.java.net;
>> hotspot-runtime-dev at openjdk.java.net Subject: Re: Request for
>> Review (XXL): 7104647: Adding a diagnostic command framework
>>
>> For the hotspot part at
>> http://cr.openjdk.java.net/~fparain/7104647/webrev.hotspot.00/
>>
>> Most of my comments are style-related.  Nice job on the
>> implementation architecture.
>>
>> In attachListener.cpp:
>>
>> You might want to delete the first "return JNI_OK;" line, since the
>> code under HAS_PENDING_EXCEPTION just falls through.
>>
>> In jmm.h:
>>
>> Be nice to indent "(JNIEnv" on lines 318, 319 and 325 the same as
>> the existing declarations.  Add a newline just before it on line
>> 322.
>>
>>
>> In diagnosticFramework.hpp:
>>
>> Fix indenting for lines 63-66, insert blank before "size_t" on line
>> 48.
>>
>> Hotspot convention is that getter method names don't include a
>> "get_" prefix. So, e.g., DCmdArgIter::get_key_addr() s/b
>> DCmdArgIter::key_addr(). Similarly, getters such as is_enabled()
>> should retrieve a field named "_is_enabled" rather than one named
>> "enabled".  You follow the "_is_enabled" convention in other places
>> such as GenDCmdArgument.  Or you could use enabled() to get the
>> value of the _enabled field.
>>
>> Also generally, I'd use accessor methods in the implementation of
>> more complex member methods rather than access the underlying
>> fields directly. E.g. in GenDCmdArgument::read_value, I'd use
>> is_set() and set_is_set(true), (set_is_set() is not actually
>> defined, but should be) rather than directly accessing _is_set.
>> Though sometimes doing this is too much of a pain with fields whose
>> type is a template argument, as in the
>> DCmdArggument<char*>::parse_value() method in
>> diagnosticArgument.cpp.
>>
>> For easy readability, it'd be nice to line up field names (the ones
>> with an _ prefix) at the same column.
>>
>> On line 200, "instanciated" ->  "instantiated"
>>
>> On line 218, I'd use "heap_allocated" rather than "heap" for the
>> formal arg name.
>>
>> On line 248, you could indent the text to start underneath
>> "outputStream". I generally find that indenting arguments lines
>> (formal or actual) so they line up with the first argument position
>> make the code more readable, but I'm not religious about it.
>>
>> On line 265, "instanciated" ->  "instantiated"
>>
>> DCmdFactorys are members of a singly-linked list, right?  If so,
>> it'd be good to have a comment to that effect on the declaration of
>> _next.
>>
>> On line 322, insert a blank before "true".  You might fix this in
>> other places where there's no blank between a comma in an argument
>> list and the following parameter value.
>>
>>
>> In diagnosticCommandFramework.cpp:
>>
>> The code from lines 80-95 and 105-120 is identical.  Factor out?
>>
>>
>> In diagnosticArgument.cpp:
>>
>> On line 41, insert blanks before the actual arguments.  (see above
>> generic comment)
>>
>> On line 77, the "if" is indented one space too many.
>>
>>
>> In management.cpp:
>>
>> I'd be consistent with having or not having a space between
>> "while", "if" and "for" and the following "(" in this and your
>> other code.  Most hotspot code has a space.
>>
>>
>> Thanks,
>>
>> Paul
>>
>>
>> On 12/2/11 8:57 AM, Frederic Parain wrote:
>>> Hi All,
>>>
>>> [Posting to serviceability-dev, runtime-dev and core-libs-dev
>>> because changes are pretty big and touch all these areas]
>>>
>>> Here's a framework for issuing diagnostics commands to the JVM.
>>> Diagnostic commands are actions executed inside the JVM mainly
>>> for monitoring or management purpose. This work has two parts.
>>> The first part is in the hotspot repository and contains the
>>> framework itself with two diagnostic commands. The second part is
>>> in the JDK repository and contains the command line utility to
>>> invoke diagnostic commands as well as the HotSpotDiagnosticMXBean
>>> extensions. The HotSpotDiagnosticMXBean extensions allow a remote
>>> client to discover and invoke diagnostic commands using a JMX
>>> connection.
>>>
>>> This changeset only contains two diagnostic commands, more
>>> commands will be added in the future, as a standalone effort to
>>> improve the monitoring and management services of the JVM, or as
>>> part of other projects.
>>>
>>> Webrevs are here:
>>> http://cr.openjdk.java.net/~fparain/7104647/webrev.hotspot.00/
>>> http://cr.openjdk.java.net/~fparain/7104647/webrev.jdk.00/
>>>
>>> Here's a technical description of this work:
>>>
>>> 1 - The Diagnostic Command Framework 1-1 Overview
>>>
>>> The diagnostic command framework is fully implemented in native
>>> code and relies on the HotSpot's internal exception mechanism.
>>> The rational of a pure native implementation is to be able to
>>> execute diagnostic commands even in critical situations like an
>>> OutOfMemory error. All diagnostic commands are registered in a
>>> single list, and two flags control the way a user can interact
>>> with them. The "hidden" flag prevents a diagnostic command from
>>> appearing in the list of available commands returned by the
>>> "help" command. However, it's still possible to get the detailed
>>> help message for a hidden command with the "help <command name>"
>>> syntax (but it requires to know the name of the hidden command).
>>> The second flag is "enabled" and it controls if a command can be
>>> invoked or not. When listed with the "help" commands, disabled
>>> commands appear with a "[disabled]" label in their description.
>>> If the user tries to invoke a disabled command, an error message
>>> is returned and the command is not run. This error message can be
>>> customized on a per command base. The framework just provides
>>> these two flags with their semantic, it doesn't provide any
>>> policy or mechanism to set or modify these flags. These actions
>>> will be delegated to the JVM or special diagnostic commands.
>>>
>>> 1-2 Implementation
>>>
>>> All diagnostic commands are implemented as subclasses of the DCmd
>>> class defined in services/diagnosticFramework.hpp. Here's the
>>> layout of the DCmd class and the list of methods that a new
>>> command has to define or overwrite:
>>>
>>> class DCmd { DCmd(outputStream *output);
>>>
>>> static const char *get_name();
>>>
>>> static const char *get_description();
>>>
>>> static const char *get_disabled_message();
>>>
>>> static const char *get_impact();
>>>
>>> static int get_num_arguments();
>>>
>>> virtual void print_help(outputStream* out);
>>>
>>> virtual void parse(CmdLine* line, char delim, TRAPS);
>>>
>>> virtual void execute(TRAPS);
>>>
>>> virtual void reset(TRAPS);
>>>
>>> virtual void cleanup();
>>>
>>> virtual GrowableArray<const char *>* get_argument_name_array();
>>>
>>> virtual GrowableArray<DCmdArgumentInfo*>*
>>> get_argument_info_array(); }
>>>
>>> A diagnostic command is always instantiated with an outputStream
>>> in parameter. This outputStream can point either to a file, a
>>> buffer or a socket (see the ostream.hpp file).
>>>
>>> The get_name() method returns the string that will identify the
>>> command (i.e. the string to put on the command line to invoke
>>> it).
>>>
>>> The get_description() methods returns the global description of
>>> the command.
>>>
>>> The get_disabled_message() returns the customized message to
>>> return when the command is disabled, without having to
>>> instantiate the command.
>>>
>>> The get_impact() returns a description of the intrusiveness of
>>> the diagnostic command on the Java Virtual Machine behavior. The
>>> rational for this method is that some diagnostic commands can
>>> seriously disrupt the behavior of the Java Virtual Machine (for
>>> instance a Thread Dump for an application with several tens of
>>> thousands of threads, or a Head Dump with a 40GB+ heap size) and
>>> other diagnostic commands have no serious impact on the JVM (for
>>> instance, getting the command line arguments or the JVM version).
>>> The recommended format for the description is<impact level>:
>>> [longer description], where the impact level is selected among
>>> this list: {low, medium, high}. The optional longer description
>>> can provide more specific details like the fact that Thread Dump
>>> impact depends on the heap size.
>>>
>>> The get_num_arguments() returns the number of options/arguments
>>> recognized by the diagnostic command. This method is only used by
>>> the JMX interface support (see below).
>>>
>>> The print_help() methods prints a detailed help on the
>>> outputStream passed in argument. The detailed help contains the
>>> list of all supported options with their type and description.
>>>
>>> The parse() method is in charge of parsing the command arguments.
>>> Each command is free to implement its own argument parser.
>>> However, an argument parser framework is provided (see section
>>> 1-3) to ease the implementation, but its use is optional. The
>>> parse method takes a delimiter character in argument, which is
>>> used to mark the limit between two arguments (typically
>>> invocation from jcmd will use a space character as a delimiter
>>> while invocation from the JVM command line parsing code will use
>>> a comma as a delimiter).
>>>
>>> The execute() method is naturally the one to invoke to get the
>>> diagnostic command executed. The parse() and the execute()
>>> methods are dissociated, so it's a possible perform the argument
>>> parsing in one thread, and delegate the execution to another
>>> thread, as long as the diagnostic command doesn't reference
>>> thread local variables. The framework allows several instances of
>>> the same diagnostic command to be executed in parallel. If for
>>> some reasons concurrent executions should not be allowed for a
>>> given diagnostic command, this is the responsibility of the
>>> diagnostic command implementor to enforce this rule, for instance
>>> by protecting the body of the execute() method with a global
>>> lock.
>>>
>>> The reset() method is used to initialize the internal fields of
>>> the diagnostic command or to reset the internal fields to their
>>> initial value to be able to re-use an already allocated
>>> diagnostic command instance.
>>>
>>> The cleanup() method is used to perform perform cleanup (like
>>> freeing of all memory allocated to store internal data). The DCmd
>>> extends the ResourceObj class, so when allocated in a
>>> ResourceArea, destructors cannot be used to perform cleanup. To
>>> ensure that cleanup is performed in all cases, it is recommended
>>> to create a DCmdMark instance for each DCmd instance. DCmdMark is
>>> a stack allocated object with a pointer to a DCmd instance. When
>>> the DCmdMark is destroyed, its destructor calls the cleanup()
>>> method of the DCmd instance it points to. If the DCmd instance
>>> has been allocated on the C-Heap, the DCmdMark will also free the
>>> memory allocated to store the DCmd instance.
>>>
>>> The get_argument_name_array() and get_argument_info_array() are
>>> related to the JMX interface of the diagnostic command framework,
>>> so they are described in section 3.
>>>
>>> 1-3 DCmdParser framework
>>>
>>> The DCmdParser class is an optional framework to help development
>>> of argument parsers. It provides many features required by the
>>> diagnostic command framework (generation of the help message or
>>> the argument descriptions for the JMX interface) but all these
>>> features can easily be re-implemented if a developer decides not
>>> to use the DCmdParser framework.
>>>
>>> The DCmdParser class is relying on the DCmdArgument template.
>>> This template must be used to define the different types of
>>> argument the parser will have to handle. When a new
>>> specialization of the template is done, three methods have to be
>>> provided:
>>>
>>> void parse_value(const char *str,size_t len,TRAPS); void
>>> init_value(TRAPS); void destroy_value();
>>>
>>> The parse_value() method is used to convert a string into an
>>> argument value. The print_value() method is used to display the
>>> default value (support for the detailed help message). The
>>> init_value() method is used to initialize or reset the argument
>>> value. The destroy_value() method is a clean-up method (useful
>>> when the argument has allocated some C-Heap memory to store its
>>> value and this memory has to be freed before destroying the
>>> DCmdArgument instance).
>>>
>>> The DCmdParser makes a distinction between options and
>>> arguments. Options are identified by a key name that must appear
>>> on the command line, while argument are identified just by the
>>> position of the argument on the command line. Options use
>>> the<key>=<value>  syntax. In case of boolean options, the
>>> '=<value>' part of the syntax can be omitted to set the option to
>>> true. Arguments are just sequences characters delimited by a
>>> separator character. This separator can be specified at runtime
>>> when invoking the diagnostic command framework. If an argument
>>> contain a character that could be used as a delimiter, it's
>>> possible to enclose the argument between single or double quotes.
>>> Options are arguments are instantiated using the same
>>> DCmdArgument class but they're registered differently to the
>>> DCmdParser.
>>>
>>> The way to use the DCmdParser is to declare the parser and the
>>> option/arguments as fields of the diagnostic command class (which
>>> is itself a sub-class of the DCmd class), like this:
>>>
>>>
>>> class EchoDCmd : public DCmd { protected: DCmdParser
>>> _dcmdparser;
>>>
>>> DCmdArgument<jlong>  _required;
>>>
>>> DCmdArgument<jlong>  _intval;
>>>
>>> DCmdArgument<bool>  _boolval;
>>>
>>> DCmdArgument<char *>  _stringval;
>>>
>>> DCmdArgument<char *>  _first_arg;
>>>
>>> DCmdArgument<jlong>  _second_arg;
>>>
>>> DCmdArgument<char *>  _optional_arg;
>>>
>>> }
>>>
>>> The parser and the options/arguments must be initialized before
>>> the diagnostic command class, and the options/arguments have to
>>> be registered to the parser like this:
>>>
>>> EchoDCmd(outputStream *output) : DCmd(output),
>>> _stringval("-strval","a string argument","STRING",false),
>>>
>>> _boolval("-boolval","a boolean argument","BOOLEAN",false),
>>>
>>> _intval("-intval","an integer argument","INTEGER",false),
>>>
>>> _required("-req","a mandatory integer argument","INTEGER",true),
>>>
>>> _fist_arg("first argument","a string argument","STRING",true),
>>>
>>> _second_arg("second argument,"an integer
>>> argument,"INTEGER",true),
>>>
>>> _optional_arg("optional argument","an optional string
>>> argument","STRING","false")
>>>
>>> {
>>>
>>> _dcmdparser.add_dcmd_option(&_stringval)
>>>
>>> _dcmdparser.add_dcmd_option(&_boolval);
>>>
>>> _dcmdparser.add_dcmd_option(&_intval);
>>>
>>> _dcmdparser.add_dcmd_option(&_required);
>>>
>>> _dcmdparser.add_argument(&_first_arg);
>>>
>>> _dcmdparser.add_argument(&_second_arg);
>>>
>>> _dcmdparser.add_argument(&_optional_arg); };
>>>
>>> The add_dcmd_argument()/ add_dcmd_option() method is used to add
>>> an argument/option to the parser. The option/argument constructor
>>> takes the name of the option/argument, its description, a string
>>> describing its type and a boolean to specify if the
>>> option/argument is mandatory or not. The parser doesn't support
>>> option/argument duplicates (having the same name) but the code
>>> currently doesn't check for duplicates.The order used to register
>>> options has no impact on the parser. However, the order used to
>>> register arguments is critical because the parser will use the
>>> same order to parse the command line. In the example above, the
>>> parser expects to have a first argument of type STRING (parsed
>>> using _first_arg), then a second argument of type INTEGER (parsed
>>> using _second_arg) and optionally a third parameter of type
>>> STRING (parsed using _optional_arg). A mandatory option or
>>> argument has to be specify every time the command is invoked. If
>>> it is missing, an exception is thrown at the end of the parsing.
>>> Optional arguments have to be registered after mandatory
>>> arguments. An optional argument will be considered for parsing
>>> only if all arguments before it (mandatory or not) have already
>>> been used to parse the command line.
>>>
>>> The DCmdParser and its DCmdArgument instances are embedded in the
>>> DCmd instance. The rational for this design is to limit the
>>> number of C-heap allocations but also to be able to pre-allocate
>>> diagnostic command instances for critical situations. If the
>>> process is running out of C-heap space, it's not possible to
>>> instantiate new diagnostic commands to troubleshoot the
>>> situation. By pre-allocating some diagnostic commands, it will be
>>> possible to run them even in this critical situation. Of course,
>>> the diagnostic command itself should not try to allocate memory
>>> during its execution, this prevents the diagnostic command to use
>>> variable length arguments like strings. By nature, pre-allocated
>>> diagnostic commands aim to be re-usable, this is the purpose of
>>> the reset() method which restores the default status of all
>>> arguments.
>>>
>>> 1-4 Internal invocation
>>>
>>> Using a diagnostic command from the JVM itself is pretty easy:
>>> instantiate the class and invoke the parse() method then the
>>> execute() method. A diagnostic command can be instantiated from
>>> inside the JVM even it is not registered. This is a difference
>>> with the external invocations (from jcmd or JMX) that require the
>>> command to be
>> registered.
>>>
>>> 2 - The JCmd interface
>>>
>>> Diagnostic commands can also be invoked from outside the JVM
>>> process, using the new 'jcmd' utility. The jcmd program uses the
>>> attach API to connect to the JVM, send requests and receive
>>> results. The jcmd utility must be launched on the same machine
>>> than the one running the JVM (its a local tool). Launched without
>>> arguments, jcmd displays a list of all JVMs running on the
>>> machine. The jcmd source code is in the jdk repository like other
>>> existing j* tools.
>>>
>>> To execute a diagnostic command in a particular JVM, the generic
>>> syntax is:
>>>
>>> jcmd<pid_of_the_jvm>  <command_name>  [arguments]
>>>
>>> The attachListener has been modified to recognized the jcmd
>>> requests. When a jcmd request is identified, it is parsed to
>>> extract the command name. The JVM performs a look up of this
>>> command in a list of registered commands. To be executable by an
>>> external request, a diagnostic command has to be registered. The
>>> registration is performed with the DCmdFactory class (see
>>> services/management.cpp).
>>>
>>> 3 - The JMX interface
>>>
>>> The framework provides a JMX based interface to the diagnostic
>>> commands. This interface allows remote invocation of diagnostic
>>> commands through a JMX connection.
>>>
>>> 3-1 The interface
>>>
>>> The information related to the diagnostic commands are
>>> accessible through new methods added to the
>>> com.sun.management.HotspotDiagnosticMXBean:
>>>
>>> public List<String>  getDiagnosticCommands();
>>>
>>> public DiagnosticCommandInfo getDiagnosticCommandInfo(String
>>> command);
>>>
>>> public List<DiagnosticCommandInfo>
>>> getDiagnosticCommandInfo(List<String> command);
>>>
>>> public List<DiagnosticCommandInfo>  getDiagnosticCommandInfo();
>>>
>>> public String execute(String commandLine) throws
>>> IllegalArgumentException ;
>>>
>>> public String execute(String cmd, String... arguments) throws
>>> IllegalArgumentException;
>>>
>>>
>>> The getDiagnosticCommands() method returns an array containing
>>> the names of the not-hidden registered diagnostic commands.
>>>
>>> The three getDiagnosticCommandInfo() methods return one or
>>> several diagnostic command descriptions using the
>>> DiagnosticCommandInfo class.
>>>
>>> The two execute() methods allow the user the invoke a diagnostic
>>> command in different ways.
>>>
>>> The DiagnosticCommandInfo class is describing a diagnostic
>>> command with the following information:
>>>
>>> public class DiagnosticCommandInfo {
>>>
>>> public String getName();
>>>
>>> public String getDescription();
>>>
>>> public String getImpact();
>>>
>>> public boolean isEnabled();
>>>
>>> public List<DiagnosticCommandArgumentInfo>  getArgumentsInfo();
>>> }
>>>
>>> The getName() method returns the name of the diagnostic command.
>>> This name is the one to use in execute() methods to invoke the
>>> diagnostic command.
>>>
>>> The getDescription() method returns a general description of the
>>> diagnostic command.
>>>
>>> The getImpact() method returns a description of the intrusiveness
>>> of diagnostic command.
>>>
>>> The isEnabled() method returns true if the method is enabled,
>>> false if it is disabled. A disabled method cannot be executed.
>>>
>>> The getArgumentsInfo() returns a list of descriptions for the
>>> options or arguments recognized by the diagnostic command. Each
>>> option/argument is described by a DiagnosticCommandArgumentInfo
>>> instance:
>>>
>>> public class DiagnosticCommandArgumentInfo {
>>>
>>> public String getName();
>>>
>>> public String getDescription();
>>>
>>> public String getType();
>>>
>>> public String getDefault();
>>>
>>> public boolean isMandatory();
>>>
>>> public boolean isOption();
>>>
>>> public int getPosition(); }
>>>
>>> If the DiagnosticCommandArgumentInfo instance describes an
>>> option, isOption() returns true and getPosition() returns -1.
>>> Otherwise, when the DiagnosticCommandArgumentInfo instance
>>> describes an argument, isOption() returns false and getPosition()
>>> returns the expected position for this argument. The position of
>>> an argument is defined relatively to all arguments passed on the
>>> command line, options are not considered when defining an
>>> argument position. The getDefault() method returns the default
>>> value of the argument if a default has been defined, otherwise it
>>> returns null.
>>>
>>> 3-2 The implementation
>>>
>>> The framework has been designed in a way that prevents
>>> diagnostic command developers to worry about the JMX interface.
>>> In addition to the methods described in section 1-2, a diagnostic
>>> command developer has to provide three methods:
>>>
>>> int get_num_arguments()
>>>
>>> which returns the number of option and arguments supported by
>>> the command.
>>>
>>> GrowableArray<const char *>* get_argument_name_array()
>>>
>>> which provides the name of the arguments supported by the
>>> command.
>>>
>>> GrowableyArray<DCmdArgumentInfo*>* get_argument_info_array()
>>>
>>> which provides the description of each argument with a
>>> DCmdArgumentInfo instance. DCmdArgumentInfo is a C++ class used
>>> by the framework to generate the
>>> sun.com.management.DcmdArgumentInfo instances. This is done
>>> automatically and the diagnostic command developer doesn't need
>>> to know how to create Java objects from the runtime.
>>>
>>> 4 - The Diagnostic Commands
>>>
>>> To avoid name collisions between diagnostic commands coming from
>>> different projects, use of a flat name space should be avoided
>>> and a more structured organization is recommended. The framework
>>> itself doesn't depend on this organization, so it will be a set
>>> of rules defining a convention in the way commands are named.
>>>
>>> Diagnostic commands can easily organized in a hierarchical way,
>>> so the template for a command name can be:
>>>
>>> <domain>.[sub-domain.]<command>
>>>
>>> This template can be extended with sub-sub-domains and so on.
>>>
>>> A special set of commands without domain will be reserved for
>>> the commands related to the diagnostic framework itself, like the
>>> "help" command.
>>>
>>>
>>> Thanks,
>>>
>>> Fred
>>>

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at Oracle.com