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

Frederic Parain frederic.parain at oracle.com
Mon Dec 12 15:56:43 UTC 2011

Minor updates: http://cr.openjdk.java.net/~fparain/7104647/webrev.hotspot.04/

Fred

On 12/12/11 03:29 PM, Frederic Parain wrote: > Hi Paul, >> Thank you for the review. > I've applied all your recommendations except the refactoring > in diagnosticCommandFramework.cpp (too few lines can be really > factored out without passing many arguments). >> New webrev is here: > http://cr.openjdk.java.net/~fparain/7104647/webrev.hotspot.03/ >> Regards, >> Fred >> On 12/ 8/11 07:26 PM, Paul Hohensee wrote: >> 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 JNIOK;" line, since the code >> under >> HASPENDINGEXCEPTION 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 "sizet" on line 48. >>>> Hotspot convention is that getter method names don't include a "get" >> prefix. >> So, e.g., DCmdArgIter::getkeyaddr() s/b DCmdArgIter::keyaddr(). >> Similarly, getters such as isenabled() should retrieve a field named >> "isenabled" >> rather than one named "enabled". You follow the "isenabled" 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::readvalue, I'd use isset() and >> setisset(true), >> (setisset() is not actually defined, but should be) rather than >> directly >> accessing isset. Though sometimes doing this is too much of a pain >> with fields whose type is a template argument, as in the >> DCmdArggument<char*>::parsevalue() 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 "heapallocated" 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 >>> " 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 *getname(); >>>>>> static const char *getdescription(); >>>>>> static const char *getdisabledmessage(); >>>>>> static const char *getimpact(); >>>>>> static int getnumarguments(); >>>>>> virtual void printhelp(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 > getargumentnamearray(); >>>>>> virtual GrowableArray<DCmdArgumentInfo*>* getargumentinfoarray(); >>> } >>>>>> 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 getname() method returns the string that will identify the command >>> (i.e. the string to put on the command line to invoke it). >>>>>> The getdescription() methods returns the global description of the >>> command. >>>>>> The getdisabledmessage() returns the customized message to return when >>> the command is disabled, without having to instantiate the command. >>>>>> The getimpact() 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 >> 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 getnumarguments() returns the number of options/arguments >>> recognized by the diagnostic command. This method is only used by the >>> JMX interface support (see below). >>>>>> The printhelp() 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 getargumentnamearray() and getargumentinfoarray() 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 parsevalue(const char *str,sizet len,TRAPS); >>> void initvalue(TRAPS); >>> void destroyvalue(); >>>>>> The parsevalue() method is used to convert a string into an argument >>> value. The printvalue() method is used to display the default value >>> (support for the detailed help message). The initvalue() method is used >>> to initialize or reset the argument value. The destroyvalue() 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 = syntax. In case of >>> boolean options, the '=' 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 required; >>>>>> DCmdArgument intval; >>>>>> DCmdArgument boolval; >>>>>> DCmdArgument<char *> stringval; >>>>>> DCmdArgument<char *> firstarg; >>>>>> DCmdArgument secondarg; >>>>>> DCmdArgument<char *> optionalarg; >>>>>> } >>>>>> 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), >>>>>> fistarg("first argument","a string argument","STRING",true), >>>>>> secondarg("second argument,"an integer argument,"INTEGER",true), >>>>>> optionalarg("optional argument","an optional string >>> argument","STRING","false") >>>>>> { >>>>>> dcmdparser.adddcmdoption(&stringval) >>>>>> dcmdparser.adddcmdoption(&boolval); >>>>>> dcmdparser.adddcmdoption(&intval); >>>>>> dcmdparser.adddcmdoption(&required); >>>>>> dcmdparser.addargument(&firstarg); >>>>>> dcmdparser.addargument(&secondarg); >>>>>> dcmdparser.addargument(&optionalarg); >>> }; >>>>>> The adddcmdargument()/ adddcmdoption() 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 >>> firstarg), then a second argument of type INTEGER (parsed using >>> secondarg) and optionally a third parameter of type STRING (parsed >>> using optionalarg). 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 [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 getDiagnosticCommands(); >>>>>> public DiagnosticCommandInfo getDiagnosticCommandInfo(String command); >>>>>> public List getDiagnosticCommandInfo(List >>> command); >>>>>> public List 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 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 getnumarguments() >>>>>> which returns the number of option and arguments supported by the >>> command. >>>>>> GrowableArray<const char > getargumentnamearray() >>>>>> which provides the name of the arguments supported by the command. >>>>>> GrowableyArray<DCmdArgumentInfo*>* getargumentinfoarray() >>>>>> 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: >>>>>> .[sub-domain.] >>>>>> 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

More information about the core-libs-dev mailing list