8303150: DCmd framework unnecessarily creates a DCmd instance on registration

Reviewed-by: fparain, stuefe, kevinw

Author: David Holmes

src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp

@@ -45,18 +45,6 @@ ClassLoaderHierarchyDCmd::ClassLoaderHierarchyDCmd(outputStream* output, bool he
   _dcmdparser.add_dcmd_option(&_fold);
 }
 
-
-int ClassLoaderHierarchyDCmd::num_arguments() {
-  ResourceMark rm;
-  ClassLoaderHierarchyDCmd* dcmd = new ClassLoaderHierarchyDCmd(nullptr, false);
-  if (dcmd != nullptr) {
-    DCmdMark mark(dcmd);
-    return dcmd->_dcmdparser.num_arguments();
-  } else {
-    return 0;
-  }
-}
-
 // Helper class for drawing the branches to the left of a node.
 class BranchTracker : public StackObj {
   //       "<x>"

src/hotspot/share/classfile/classLoaderHierarchyDCmd.hpp

@@ -51,7 +51,7 @@ class ClassLoaderHierarchyDCmd: public DCmdWithParser {
                         "monitor", nullptr};
     return p;
   }
-  static int num_arguments();
+  static int num_arguments() { return 3; }
   virtual void execute(DCmdSource source, TRAPS);
 
 };

src/hotspot/share/jfr/dcmd/jfrDcmds.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -447,16 +447,6 @@ void JfrConfigureFlightRecorderDCmd::print_help(const char* name) const {
   out->print_cr("");
 }
 
-int JfrConfigureFlightRecorderDCmd::num_arguments() {
-  ResourceMark rm;
-  JfrConfigureFlightRecorderDCmd* dcmd = new JfrConfigureFlightRecorderDCmd(NULL, false);
-  if (dcmd != NULL) {
-    DCmdMark mark(dcmd);
-    return dcmd->_dcmdparser.num_arguments();
-  }
-  return 0;
-}
-
 void JfrConfigureFlightRecorderDCmd::execute(DCmdSource source, TRAPS) {
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(THREAD));
 

src/hotspot/share/jfr/dcmd/jfrDcmds.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -177,7 +177,7 @@ class JfrConfigureFlightRecorderDCmd : public DCmdWithParser {
     JavaPermission p = {"java.lang.management.ManagementPermission", "monitor", NULL};
     return p;
   }
-  static int num_arguments();
+  static int num_arguments() { return 9; }
   virtual void execute(DCmdSource source, TRAPS);
   virtual void print_help(const char* name) const;
 };

src/hotspot/share/logging/logDiagnosticCommand.cpp

@@ -45,17 +45,6 @@ LogDiagnosticCommand::LogDiagnosticCommand(outputStream* output, bool heap_alloc
   _dcmdparser.add_dcmd_option(&_rotate);
 }
 
-int LogDiagnosticCommand::num_arguments() {
-  ResourceMark rm;
-  LogDiagnosticCommand* dcmd = new LogDiagnosticCommand(nullptr, false);
-  if (dcmd != nullptr) {
-    DCmdMark mark(dcmd);
-    return dcmd->_dcmdparser.num_arguments();
-  } else {
-    return 0;
-  }
-}
-
 void LogDiagnosticCommand::registerCommand() {
   uint32_t full_visibility = DCmd_Source_Internal | DCmd_Source_AttachAPI | DCmd_Source_MBean;
   DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<LogDiagnosticCommand>(full_visibility, true, false));

src/hotspot/share/logging/logDiagnosticCommand.hpp

@@ -49,7 +49,7 @@ class LogDiagnosticCommand : public DCmdWithParser {
   LogDiagnosticCommand(outputStream* output, bool heap_allocated);
   void execute(DCmdSource source, TRAPS);
   static void registerCommand();
-  static int num_arguments();
+  static int num_arguments() { return 7; }
 
   static const char* name() {
     return "VM.log";

src/hotspot/share/memory/metaspace/metaspaceDCmd.cpp

@@ -58,17 +58,6 @@ MetaspaceDCmd::MetaspaceDCmd(outputStream* output, bool heap) :
   _dcmdparser.add_dcmd_option(&_scale);
 }
 
-int MetaspaceDCmd::num_arguments() {
-  ResourceMark rm;
-  MetaspaceDCmd* dcmd = new MetaspaceDCmd(nullptr, false);
-  if (dcmd != nullptr) {
-    DCmdMark mark(dcmd);
-    return dcmd->_dcmdparser.num_arguments();
-  } else {
-    return 0;
-  }
-}
-
 void MetaspaceDCmd::execute(DCmdSource source, TRAPS) {
   // Parse scale value.
   const char* scale_value = _scale.value();

src/hotspot/share/memory/metaspace/metaspaceDCmd.hpp

@@ -57,7 +57,7 @@ class MetaspaceDCmd : public DCmdWithParser {
                         "monitor", nullptr};
     return p;
   }
-  static int num_arguments();
+  static int num_arguments() { return 8; }
   virtual void execute(DCmdSource source, TRAPS);
 };
 

src/hotspot/share/services/diagnosticCommand.cpp

@@ -1073,17 +1073,6 @@ ThreadDumpToFileDCmd::ThreadDumpToFileDCmd(outputStream* output, bool heap) :
   _dcmdparser.add_dcmd_argument(&_filepath);
 }
 
-int ThreadDumpToFileDCmd::num_arguments() {
-  ResourceMark rm;
-  ThreadDumpToFileDCmd* dcmd = new ThreadDumpToFileDCmd(nullptr, false);
-  if (dcmd != nullptr) {
-    DCmdMark mark(dcmd);
-    return dcmd->_dcmdparser.num_arguments();
-  } else {
-    return 0;
-  }
-}
-
 void ThreadDumpToFileDCmd::execute(DCmdSource source, TRAPS) {
   bool json = (_format.value() != nullptr) && (strcmp(_format.value(), "json") == 0);
   char* path = _filepath.value();

src/hotspot/share/services/diagnosticCommand.hpp

@@ -44,6 +44,7 @@ class HelpDCmd : public DCmdWithParser {
   DCmdArgument<bool> _all;
   DCmdArgument<char*> _cmd;
 public:
+  static int num_arguments() { return 2; }
   HelpDCmd(outputStream* output, bool heap);
   static const char* name() { return "help"; }
   static const char* description() {
@@ -113,6 +114,7 @@ class PrintVMFlagsDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<bool> _all;
 public:
+  static int num_arguments() { return 1; }
   PrintVMFlagsDCmd(outputStream* output, bool heap);
   static const char* name() { return "VM.flags"; }
   static const char* description() {
@@ -135,6 +137,7 @@ class SetVMFlagDCmd : public DCmdWithParser {
   DCmdArgument<char*> _value;
 
 public:
+  static int num_arguments() { return 2; }
   SetVMFlagDCmd(outputStream* output, bool heap);
   static const char* name() { return "VM.set_flag"; }
   static const char* description() {
@@ -176,6 +179,7 @@ class JVMTIAgentLoadDCmd : public DCmdWithParser {
   DCmdArgument<char*> _libpath;
   DCmdArgument<char*> _option;
 public:
+  static int num_arguments() { return 2; }
   JVMTIAgentLoadDCmd(outputStream* output, bool heap);
   static const char* name() { return "JVMTI.agent_load"; }
   static const char* description() {
@@ -216,6 +220,7 @@ class VMUptimeDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<bool> _date;
 public:
+  static int num_arguments() { return 1; }
   VMUptimeDCmd(outputStream* output, bool heap);
   static const char* name() { return "VM.uptime"; }
   static const char* description() {
@@ -316,6 +321,7 @@ class HeapDumpDCmd : public DCmdWithParser {
   DCmdArgument<jlong> _gzip;
   DCmdArgument<bool> _overwrite;
 public:
+  static int num_arguments() { return 4; }
   HeapDumpDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "GC.heap_dump";
@@ -342,6 +348,7 @@ class ClassHistogramDCmd : public DCmdWithParser {
   DCmdArgument<bool> _all;
   DCmdArgument<jlong> _parallel_thread_num;
 public:
+  static int num_arguments() { return 2; }
   ClassHistogramDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "GC.class_histogram";
@@ -366,6 +373,7 @@ class ClassHierarchyDCmd : public DCmdWithParser {
   DCmdArgument<bool> _print_subclasses; // true if subclasses of the specified classname should be printed.
   DCmdArgument<char*> _classname; // Optional single class name whose hierarchy should be printed.
 public:
+  static int num_arguments() { return 3; }
   ClassHierarchyDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.class_hierarchy";
@@ -392,6 +400,7 @@ class DumpSharedArchiveDCmd: public DCmdWithParser {
   DCmdArgument<char*> _suboption;   // option of VM.cds
   DCmdArgument<char*> _filename;    // file name, optional
 public:
+  static int num_arguments() { return 2; }
   DumpSharedArchiveDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.cds";
@@ -407,7 +416,6 @@ class DumpSharedArchiveDCmd: public DCmdWithParser {
                         "monitor", nullptr};
     return p;
   }
-  static int num_arguments();
   virtual void execute(DCmdSource source, TRAPS);
 };
 #endif // INCLUDE_CDS
@@ -418,6 +426,7 @@ class ThreadDumpDCmd : public DCmdWithParser {
   DCmdArgument<bool> _locks;
   DCmdArgument<bool> _extended;
 public:
+  static int num_arguments() { return 2; }
   ThreadDumpDCmd(outputStream* output, bool heap);
   static const char* name() { return "Thread.print"; }
   static const char* description() {
@@ -469,6 +478,8 @@ class JMXStartRemoteDCmd : public DCmdWithParser {
   DCmdArgument<char *> _jdp_name;
 
 public:
+  static int num_arguments() { return 21; }
+
   JMXStartRemoteDCmd(outputStream *output, bool heap_allocated);
 
   static const char *name() {
@@ -632,6 +643,7 @@ class CodeHeapAnalyticsDCmd : public DCmdWithParser {
   DCmdArgument<char*> _function;
   DCmdArgument<jlong> _granularity;
 public:
+  static int num_arguments() { return 2; }
   CodeHeapAnalyticsDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "Compiler.CodeHeap_Analytics";
@@ -696,6 +708,7 @@ class CompilerDirectivesAddDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<char*> _filename;
 public:
+  static int num_arguments() { return 1; }
   CompilerDirectivesAddDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "Compiler.directives_add";
@@ -781,6 +794,7 @@ class SymboltableDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<bool> _verbose;
 public:
+  static int num_arguments() { return 1; }
   SymboltableDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.symboltable";
@@ -803,6 +817,7 @@ class StringtableDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<bool> _verbose;
 public:
+  static int num_arguments() { return 1; }
   StringtableDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.stringtable";
@@ -825,6 +840,7 @@ class SystemDictionaryDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<bool> _verbose;
 public:
+  static int num_arguments() { return 1; }
   SystemDictionaryDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.systemdictionary";
@@ -847,6 +863,7 @@ class ClassesDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<bool> _verbose;
 public:
+  static int num_arguments() { return 1; }
   ClassesDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.classes";
@@ -891,6 +908,7 @@ class EventLogDCmd : public DCmdWithParser {
   DCmdArgument<char*> _log;
   DCmdArgument<char*> _max;
 public:
+  static int num_arguments() { return 2; }
   EventLogDCmd(outputStream* output, bool heap);
   static const char* name() {
     return "VM.events";
@@ -917,6 +935,7 @@ class ThreadDumpToFileDCmd : public DCmdWithParser {
   DCmdArgument<char*> _format;
   DCmdArgument<char*> _filepath;
 public:
+  static int num_arguments() { return 3; }
   ThreadDumpToFileDCmd(outputStream *output, bool heap);
   static const char *name() {
     return "Thread.dump_to_file";
@@ -931,7 +950,6 @@ class ThreadDumpToFileDCmd : public DCmdWithParser {
     JavaPermission p = {"java.lang.management.ManagementPermission", "monitor", nullptr};
     return p;
   }
-  static int num_arguments();
   virtual void execute(DCmdSource source, TRAPS);
 };
 

src/hotspot/share/services/diagnosticFramework.hpp

@@ -276,9 +276,7 @@ class DCmd : public AnyObj {
     return p;
   }
   // num_arguments() is used by the DCmdFactoryImpl::get_num_arguments() template functions.
-  // - For subclasses of DCmdWithParser, it's calculated by DCmdParser::num_arguments().
-  // - Other subclasses of DCmd have zero arguments by default. You can change this
-  //   by defining your own version of MyDCmd::num_arguments().
+  // All subclasses should override this to report the actual number of arguments.
   static int num_arguments()        { return 0; }
   outputStream* output() const      { return _output; }
   bool is_heap_allocated() const    { return _is_heap_allocated; }
@@ -439,13 +437,14 @@ template <class DCmdClass> class DCmdFactoryImpl : public DCmdFactory {
   }
 
 private:
+#ifdef ASSERT
   template <typename T, ENABLE_IF(!std::is_base_of<DCmdWithParser, T>::value)>
-  static int get_num_arguments() {
+  static int get_parsed_num_arguments() {
     return T::num_arguments();
   }
 
   template <typename T, ENABLE_IF(std::is_base_of<DCmdWithParser, T>::value)>
-  static int get_num_arguments() {
+  static int get_parsed_num_arguments() {
     ResourceMark rm;
     DCmdClass* dcmd = new DCmdClass(nullptr, false);
     if (dcmd != nullptr) {
@@ -455,6 +454,20 @@ template <class DCmdClass> class DCmdFactoryImpl : public DCmdFactory {
       return 0;
     }
   }
+#endif
+
+  template <typename T, ENABLE_IF(std::is_convertible<T, DCmd>::value)>
+  static int get_num_arguments() {
+    int n_args = T::num_arguments();
+#ifdef ASSERT
+    int n_parsed_args = get_parsed_num_arguments<T>();
+    assert(n_args == n_parsed_args,
+           "static argument count %d does not match parsed argument count %d",
+           n_args, n_parsed_args);
+#endif
+    return n_args;
+  }
+
 };
 
 #endif // SHARE_SERVICES_DIAGNOSTICFRAMEWORK_HPP

src/hotspot/share/services/nmtDCmd.hpp

@@ -44,6 +44,7 @@ class NMTDCmd: public DCmdWithParser {
   DCmdArgument<char*> _scale;
 
  public:
+  static int num_arguments() { return 7; }
   NMTDCmd(outputStream* output, bool heap);
   static const char* name() { return "VM.native_memory"; }
   static const char* description() {