|
| 1 | +From f8df953e61372b396f351403ff6ba165720881bb Mon Sep 17 00:00:00 2001 |
| 2 | +From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= <mgronlun@openjdk.org> |
| 3 | +Date: Mon, 21 Jun 2021 11:16:41 +0000 |
| 4 | +Subject: [PATCH] 8268702: JFR diagnostic commands lack argument descriptors |
| 5 | + when viewed using Platform MBean Server |
| 6 | + |
| 7 | +Reviewed-by: egahlin |
| 8 | +--- |
| 9 | + src/hotspot/share/jfr/dcmd/jfrDcmds.cpp | 94 +++++++++++++++++-- |
| 10 | + src/hotspot/share/jfr/dcmd/jfrDcmds.hpp | 12 +++ |
| 11 | + .../jdk/jfr/internal/dcmd/AbstractDCmd.java | 5 +- |
| 12 | + 3 files changed, 101 insertions(+), 10 deletions(-) |
| 13 | + |
| 14 | +diff --git a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp |
| 15 | +index f821de98a51..d5d1aca378e 100644 |
| 16 | +--- a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp |
| 17 | ++++ b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp |
| 18 | +@@ -23,7 +23,7 @@ |
| 19 | + */ |
| 20 | + |
| 21 | + #include "precompiled.hpp" |
| 22 | +-#include "classfile/javaClasses.hpp" |
| 23 | ++#include "classfile/javaClasses.inline.hpp" |
| 24 | + #include "classfile/vmSymbols.hpp" |
| 25 | + #include "jfr/jfr.hpp" |
| 26 | + #include "jfr/dcmd/jfrDcmds.hpp" |
| 27 | +@@ -33,6 +33,7 @@ |
| 28 | + #include "logging/log.hpp" |
| 29 | + #include "logging/logConfiguration.hpp" |
| 30 | + #include "logging/logMessage.hpp" |
| 31 | ++#include "memory/arena.hpp" |
| 32 | + #include "memory/resourceArea.hpp" |
| 33 | + #include "oops/objArrayOop.inline.hpp" |
| 34 | + #include "oops/oop.inline.hpp" |
| 35 | +@@ -219,7 +220,6 @@ void JfrDCmd::invoke(JfrJavaArguments& method, TRAPS) const { |
| 36 | + JfrJavaArguments constructor_args(&constructor_result); |
| 37 | + constructor_args.set_klass(javaClass(), CHECK); |
| 38 | + |
| 39 | +- ResourceMark rm(THREAD); |
| 40 | + HandleMark hm(THREAD); |
| 41 | + JNIHandleBlockManager jni_handle_management(THREAD); |
| 42 | + |
| 43 | +@@ -241,12 +241,10 @@ void JfrDCmd::parse(CmdLine* line, char delim, TRAPS) { |
| 44 | + } |
| 45 | + |
| 46 | + void JfrDCmd::execute(DCmdSource source, TRAPS) { |
| 47 | +- static const char signature[] = "(Ljava/lang/String;Ljava/lang/String;C)[Ljava/lang/String;"; |
| 48 | +- |
| 49 | + if (invalid_state(output(), THREAD)) { |
| 50 | + return; |
| 51 | + } |
| 52 | +- |
| 53 | ++ static const char signature[] = "(Ljava/lang/String;Ljava/lang/String;C)[Ljava/lang/String;"; |
| 54 | + JavaValue result(T_OBJECT); |
| 55 | + JfrJavaArguments execute(&result, javaClass(), "execute", signature, CHECK); |
| 56 | + jstring argument = JfrJavaSupport::new_string(_args, CHECK); |
| 57 | +@@ -271,13 +269,92 @@ void JfrDCmd::print_help(const char* name) const { |
| 58 | + static const char signature[] = "()[Ljava/lang/String;"; |
| 59 | + JavaThread* thread = JavaThread::current(); |
| 60 | + JavaValue result(T_OBJECT); |
| 61 | +- JfrJavaArguments print_help(&result, javaClass(), "printHelp", signature, thread); |
| 62 | +- invoke(print_help, thread); |
| 63 | ++ JfrJavaArguments printHelp(&result, javaClass(), "printHelp", signature, thread); |
| 64 | ++ invoke(printHelp, thread); |
| 65 | + handle_dcmd_result(output(), result.get_oop(), DCmd_Source_MBean, thread); |
| 66 | + } |
| 67 | + |
| 68 | ++// Since the DcmdFramework does not support dynamically allocated strings, |
| 69 | ++// we keep them in a thread local arena. The arena is reset between invocations. |
| 70 | ++static THREAD_LOCAL Arena* dcmd_arena = NULL; |
| 71 | ++ |
| 72 | ++static void prepare_dcmd_string_arena() { |
| 73 | ++ if (dcmd_arena == NULL) { |
| 74 | ++ dcmd_arena = new (mtTracing) Arena(mtTracing); |
| 75 | ++ } else { |
| 76 | ++ dcmd_arena->destruct_contents(); // will grow on next allocation |
| 77 | ++ } |
| 78 | ++} |
| 79 | ++ |
| 80 | ++static char* dcmd_arena_allocate(size_t size) { |
| 81 | ++ assert(dcmd_arena != NULL, "invariant"); |
| 82 | ++ return (char*)dcmd_arena->Amalloc(size); |
| 83 | ++} |
| 84 | ++ |
| 85 | ++static const char* get_as_dcmd_arena_string(oop string, JavaThread* t) { |
| 86 | ++ char* str = NULL; |
| 87 | ++ const typeArrayOop value = java_lang_String::value(string); |
| 88 | ++ if (value != NULL) { |
| 89 | ++ const size_t length = static_cast<size_t>(java_lang_String::utf8_length(string, value)) + 1; |
| 90 | ++ str = dcmd_arena_allocate(length); |
| 91 | ++ assert(str != NULL, "invariant"); |
| 92 | ++ java_lang_String::as_utf8_string(string, value, str, static_cast<int>(length)); |
| 93 | ++ } |
| 94 | ++ return str; |
| 95 | ++} |
| 96 | ++ |
| 97 | ++static const char* read_string_field(oop argument, const char* field_name, TRAPS) { |
| 98 | ++ JavaValue result(T_OBJECT); |
| 99 | ++ JfrJavaArguments args(&result); |
| 100 | ++ args.set_klass(argument->klass()); |
| 101 | ++ args.set_name(field_name); |
| 102 | ++ args.set_signature("Ljava/lang/String;"); |
| 103 | ++ args.set_receiver(argument); |
| 104 | ++ JfrJavaSupport::get_field(&args, THREAD); |
| 105 | ++ const oop string_oop = result.get_oop(); |
| 106 | ++ return string_oop != NULL ? get_as_dcmd_arena_string(string_oop, (JavaThread*)THREAD) : NULL; |
| 107 | ++} |
| 108 | ++ |
| 109 | ++static bool read_boolean_field(oop argument, const char* field_name, TRAPS) { |
| 110 | ++ JavaValue result(T_BOOLEAN); |
| 111 | ++ JfrJavaArguments args(&result); |
| 112 | ++ args.set_klass(argument->klass()); |
| 113 | ++ args.set_name(field_name); |
| 114 | ++ args.set_signature("Z"); |
| 115 | ++ args.set_receiver(argument); |
| 116 | ++ JfrJavaSupport::get_field(&args, THREAD); |
| 117 | ++ return (result.get_jint() & 1) == 1; |
| 118 | ++} |
| 119 | ++ |
| 120 | ++static DCmdArgumentInfo* create_info(oop argument, TRAPS) { |
| 121 | ++ return new DCmdArgumentInfo( |
| 122 | ++ read_string_field(argument, "name", THREAD), |
| 123 | ++ read_string_field(argument, "description", THREAD), |
| 124 | ++ read_string_field(argument, "type", THREAD), |
| 125 | ++ read_string_field(argument, "defaultValue", THREAD), |
| 126 | ++ read_boolean_field(argument, "mandatory", THREAD), |
| 127 | ++ true, // a DcmdFramework "option" |
| 128 | ++ read_boolean_field(argument, "allowMultiple", THREAD)); |
| 129 | ++} |
| 130 | ++ |
| 131 | + GrowableArray<DCmdArgumentInfo*>* JfrDCmd::argument_info_array() const { |
| 132 | +- return new GrowableArray<DCmdArgumentInfo*>(); |
| 133 | ++ static const char signature[] = "()[Ljdk/jfr/internal/dcmd/Argument;"; |
| 134 | ++ JavaThread* thread = JavaThread::current(); |
| 135 | ++ JavaValue result(T_OBJECT); |
| 136 | ++ JfrJavaArguments getArgumentInfos(&result, javaClass(), "getArgumentInfos", signature, thread); |
| 137 | ++ invoke(getArgumentInfos, thread); |
| 138 | ++ objArrayOop arguments = objArrayOop(result.get_oop()); |
| 139 | ++ assert(arguments != NULL, "invariant"); |
| 140 | ++ assert(arguments->is_array(), "must be array"); |
| 141 | ++ GrowableArray<DCmdArgumentInfo*>* const array = new GrowableArray<DCmdArgumentInfo*>(); |
| 142 | ++ const int length = arguments->length(); |
| 143 | ++ prepare_dcmd_string_arena(); |
| 144 | ++ for (int i = 0; i < length; ++i) { |
| 145 | ++ DCmdArgumentInfo* const dai = create_info(arguments->obj_at(i), thread); |
| 146 | ++ assert(dai != NULL, "invariant"); |
| 147 | ++ array->append(dai); |
| 148 | ++ } |
| 149 | ++ return array; |
| 150 | + } |
| 151 | + |
| 152 | + GrowableArray<const char*>* JfrDCmd::argument_name_array() const { |
| 153 | +@@ -387,7 +464,6 @@ void JfrConfigureFlightRecorderDCmd::execute(DCmdSource source, TRAPS) { |
| 154 | + return; |
| 155 | + } |
| 156 | + |
| 157 | +- ResourceMark rm(THREAD); |
| 158 | + HandleMark hm(THREAD); |
| 159 | + JNIHandleBlockManager jni_handle_management(THREAD); |
| 160 | + |
| 161 | +diff --git a/src/hotspot/share/jfr/dcmd/jfrDcmds.hpp b/src/hotspot/share/jfr/dcmd/jfrDcmds.hpp |
| 162 | +index 605c4033c5e..dce5205854b 100644 |
| 163 | +--- a/src/hotspot/share/jfr/dcmd/jfrDcmds.hpp |
| 164 | ++++ b/src/hotspot/share/jfr/dcmd/jfrDcmds.hpp |
| 165 | +@@ -65,6 +65,9 @@ class JfrStartFlightRecordingDCmd : public JfrDCmd { |
| 166 | + virtual const char* javaClass() const { |
| 167 | + return "jdk/jfr/internal/dcmd/DCmdStart"; |
| 168 | + } |
| 169 | ++ static int num_arguments() { |
| 170 | ++ return 11; |
| 171 | ++ } |
| 172 | + }; |
| 173 | + |
| 174 | + class JfrDumpFlightRecordingDCmd : public JfrDCmd { |
| 175 | +@@ -87,6 +90,9 @@ class JfrDumpFlightRecordingDCmd : public JfrDCmd { |
| 176 | + virtual const char* javaClass() const { |
| 177 | + return "jdk/jfr/internal/dcmd/DCmdDump"; |
| 178 | + } |
| 179 | ++ static int num_arguments() { |
| 180 | ++ return 7; |
| 181 | ++ } |
| 182 | + }; |
| 183 | + |
| 184 | + class JfrCheckFlightRecordingDCmd : public JfrDCmd { |
| 185 | +@@ -109,6 +115,9 @@ class JfrCheckFlightRecordingDCmd : public JfrDCmd { |
| 186 | + virtual const char* javaClass() const { |
| 187 | + return "jdk/jfr/internal/dcmd/DCmdCheck"; |
| 188 | + } |
| 189 | ++ static int num_arguments() { |
| 190 | ++ return 2; |
| 191 | ++ } |
| 192 | + }; |
| 193 | + |
| 194 | + class JfrStopFlightRecordingDCmd : public JfrDCmd { |
| 195 | +@@ -131,6 +140,9 @@ class JfrStopFlightRecordingDCmd : public JfrDCmd { |
| 196 | + virtual const char* javaClass() const { |
| 197 | + return "jdk/jfr/internal/dcmd/DCmdStop"; |
| 198 | + } |
| 199 | ++ static int num_arguments() { |
| 200 | ++ return 2; |
| 201 | ++ } |
| 202 | + }; |
| 203 | + |
| 204 | + class JfrConfigureFlightRecorderDCmd : public DCmdWithParser { |
| 205 | +diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/AbstractDCmd.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/AbstractDCmd.java |
| 206 | +index dda13ef67bb..9949ca3689e 100644 |
| 207 | +--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/AbstractDCmd.java |
| 208 | ++++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/AbstractDCmd.java |
| 209 | +@@ -58,7 +58,10 @@ abstract class AbstractDCmd { |
| 210 | + // Called by native |
| 211 | + public abstract String[] printHelp(); |
| 212 | + |
| 213 | +- // Called by native |
| 214 | ++ // Called by native. The number of arguments for each command is |
| 215 | ++ // reported to the DCmdFramework as a hardcoded number in native. |
| 216 | ++ // This is to avoid an upcall as part of DcmdFramework enumerating existing commands. |
| 217 | ++ // Remember to keep the two sides in synch. |
| 218 | + public abstract Argument[] getArgumentInfos(); |
| 219 | + |
| 220 | + // Called by native |
| 221 | +-- |
| 222 | +2.43.2 |
| 223 | + |
0 commit comments