8245167: Top package in method profiling shows null in JMC

Reviewed-by: neugens
Contributed-by: asemenov@azul.com

src/share/vm/classfile/classLoader.cpp

@@ -162,6 +162,64 @@ bool string_ends_with(const char* str, const char* str_to_find) {
   return (strncmp(str + (str_len - str_to_find_len), str_to_find, str_to_find_len) == 0);
 }
 
+// Used to obtain the package name from a fully qualified class name.
+// It is the responsibility of the caller to establish a ResourceMark.
+const char* ClassLoader::package_from_name(const char* const class_name, bool* bad_class_name) {
+  if (class_name == NULL) {
+    if (bad_class_name != NULL) {
+      *bad_class_name = true;
+    }
+    return NULL;
+  }
+
+  if (bad_class_name != NULL) {
+    *bad_class_name = false;
+  }
+
+  const char* const last_slash = strrchr(class_name, '/');
+  if (last_slash == NULL) {
+    // No package name
+    return NULL;
+  }
+
+  char* class_name_ptr = (char*) class_name;
+  // Skip over '['s
+  if (*class_name_ptr == '[') {
+    do {
+      class_name_ptr++;
+    } while (*class_name_ptr == '[');
+
+    // Fully qualified class names should not contain a 'L'.
+    // Set bad_class_name to true to indicate that the package name
+    // could not be obtained due to an error condition.
+    // In this situation, is_same_class_package returns false.
+    if (*class_name_ptr == 'L') {
+      if (bad_class_name != NULL) {
+        *bad_class_name = true;
+      }
+      return NULL;
+    }
+  }
+
+  int length = last_slash - class_name_ptr;
+
+  // A class name could have just the slash character in the name.
+  if (length <= 0) {
+    // No package name
+    if (bad_class_name != NULL) {
+      *bad_class_name = true;
+    }
+    return NULL;
+  }
+
+  // drop name after last slash (including slash)
+  // Ex., "java/lang/String.class" => "java/lang"
+  char* pkg_name = NEW_RESOURCE_ARRAY(char, length + 1);
+  strncpy(pkg_name, class_name_ptr, length);
+  *(pkg_name+length) = '\0';
+
+  return (const char *)pkg_name;
+}
 
 MetaIndex::MetaIndex(char** meta_package_names, int num_meta_package_names) {
   if (num_meta_package_names == 0) {

src/share/vm/classfile/classLoader.hpp

@@ -366,6 +366,11 @@ class ClassLoader: AllStatic {
   // creates a class path zip entry (returns NULL if JAR file cannot be opened)
   static ClassPathZipEntry* create_class_path_zip_entry(const char *apath);
 
+  // obtain package name from a fully qualified class name
+  // *bad_class_name is set to true if there's a problem with parsing class_name, to
+  // distinguish from a class_name with no package name, as both cases have a NULL return value
+  static const char* package_from_name(const char* const class_name, bool* bad_class_name = NULL);
+
   // Debugging
   static void verify()              PRODUCT_RETURN;
 

src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp

@@ -52,6 +52,7 @@ static u8 checkpoint_id = 0;
 // creates a unique id by combining a checkpoint relative symbol id (2^24)
 // with the current checkpoint id (2^40)
 #define CREATE_SYMBOL_ID(sym_id) (((u8)((checkpoint_id << 24) | sym_id)))
+#define CREATE_PACKAGE_ID(pkg_id) (((u8)((checkpoint_id << 24) | pkg_id)))
 
 typedef const Klass* KlassPtr;
 // XXX typedef const PackageEntry* PkgPtr;
@@ -61,12 +62,23 @@ typedef const Symbol* SymbolPtr;
 typedef const JfrSymbolId::SymbolEntry* SymbolEntryPtr;
 typedef const JfrSymbolId::CStringEntry* CStringEntryPtr;
 
-// XXX
-// static traceid package_id(KlassPtr klass) {
-//   assert(klass != NULL, "invariant");
-//   PkgPtr pkg_entry = klass->package();
-//   return pkg_entry == NULL ? 0 : TRACE_ID(pkg_entry);
-// }
+inline uintptr_t package_name_hash(const char *s) {
+  uintptr_t val = 0;
+  while (*s != 0) {
+    val = *s++ + 31 * val;
+  }
+  return val;
+}
+
+static traceid package_id(KlassPtr klass, JfrArtifactSet* artifacts) {
+  assert(klass != NULL, "invariant");
+  char* klass_name = klass->name()->as_C_string(); // uses ResourceMark declared in JfrTypeSet::serialize()
+  const char* pkg_name = ClassLoader::package_from_name(klass_name, NULL);
+  if (pkg_name == NULL) {
+    return 0;
+  }
+  return CREATE_PACKAGE_ID(artifacts->markPackage(pkg_name, package_name_hash(pkg_name)));
+}
 
 static traceid cld_id(CldPtr cld) {
   assert(cld != NULL, "invariant");
@@ -125,7 +137,7 @@ int write__artifact__klass(JfrCheckpointWriter* writer, JfrArtifactSet* artifact
     theklass = obj_arr_klass->bottom_klass();
   }
   if (theklass->oop_is_instance()) {
-    pkg_id = 0; // XXX package_id(theklass);
+    pkg_id = package_id(theklass, artifacts);
   } else {
     assert(theklass->oop_is_typeArray(), "invariant");
   }
@@ -169,28 +181,19 @@ int write__artifact__method(JfrCheckpointWriter* writer, JfrArtifactSet* artifac
 typedef JfrArtifactWriterImplHost<MethodPtr, write__artifact__method> MethodWriterImplTarget;
 typedef JfrArtifactWriterHost<MethodWriterImplTarget, TYPE_METHOD> MethodWriterImpl;
 
-// XXX
-// int write__artifact__package(JfrCheckpointWriter* writer, JfrArtifactSet* artifacts, const void* p) {
-//   assert(writer != NULL, "invariant");
-//   assert(artifacts != NULL, "invariant");
-//   assert(p != NULL, "invariant");
-//   PkgPtr pkg = (PkgPtr)p;
-//   Symbol* const pkg_name = pkg->name();
-//   const traceid package_name_symbol_id = pkg_name != NULL ? artifacts->mark(pkg_name) : 0;
-//   assert(package_name_symbol_id > 0, "invariant");
-//   writer->write((traceid)TRACE_ID(pkg));
-//   writer->write((traceid)CREATE_SYMBOL_ID(package_name_symbol_id));
-//   writer->write((bool)pkg->is_exported());
-//   return 1;
-// }
-
-// typedef LeakPredicate<PkgPtr> LeakPackagePredicate;
-// int _compare_pkg_ptr_(PkgPtr const& lhs, PkgPtr const& rhs) { return lhs > rhs ? 1 : (lhs < rhs) ? -1 : 0; }
-// typedef UniquePredicate<PkgPtr, _compare_pkg_ptr_> PackagePredicate;
-// typedef JfrPredicatedArtifactWriterImplHost<PkgPtr, LeakPackagePredicate, write__artifact__package> LeakPackageWriterImpl;
-// typedef JfrPredicatedArtifactWriterImplHost<PkgPtr, PackagePredicate, write__artifact__package> PackageWriterImpl;
-// typedef JfrArtifactWriterHost<LeakPackageWriterImpl, TYPE_PACKAGE> LeakPackageWriter;
-// typedef JfrArtifactWriterHost<PackageWriterImpl, TYPE_PACKAGE> PackageWriter;
+int write__artifact__package(JfrCheckpointWriter* writer, JfrArtifactSet* artifacts, const void* p) {
+  assert(writer != NULL, "invariant");
+  assert(artifacts != NULL, "invariant");
+  assert(p != NULL, "invariant");
+
+  CStringEntryPtr entry = (CStringEntryPtr)p;
+  const traceid package_name_symbol_id = artifacts->mark(entry->value(), package_name_hash(entry->value()));
+  assert(package_name_symbol_id > 0, "invariant");
+  writer->write((traceid)CREATE_PACKAGE_ID(entry->id()));
+  writer->write((traceid)CREATE_SYMBOL_ID(package_name_symbol_id));
+  writer->write((bool)true); // exported
+  return 1;
+}
 
 int write__artifact__classloader(JfrCheckpointWriter* writer, JfrArtifactSet* artifacts, const void* c) {
   assert(c != NULL, "invariant");
@@ -525,99 +528,17 @@ void JfrTypeSet::write_klass_constants(JfrCheckpointWriter* writer, JfrCheckpoin
   do_klasses();
 }
 
-// XXX
-// typedef CompositeFunctor<PkgPtr,
-//                          PackageWriter,
-//                          ClearArtifact<PkgPtr> > PackageWriterWithClear;
-
-// typedef CompositeFunctor<PkgPtr,
-//                          LeakPackageWriter,
-//                          PackageWriter> CompositePackageWriter;
-
-// typedef CompositeFunctor<PkgPtr,
-//                          CompositePackageWriter,
-//                          ClearArtifact<PkgPtr> > CompositePackageWriterWithClear;
-
-// class PackageFieldSelector {
-//  public:
-//   typedef PkgPtr TypePtr;
-//   static TypePtr select(KlassPtr klass) {
-//     assert(klass != NULL, "invariant");
-//     return ((InstanceKlass*)klass)->package();
-//   }
-// };
-
-// typedef KlassToFieldEnvelope<PackageFieldSelector,
-//                              PackageWriterWithClear> KlassPackageWriterWithClear;
-
-// typedef KlassToFieldEnvelope<PackageFieldSelector,
-//                              CompositePackageWriterWithClear> KlassCompositePackageWriterWithClear;
-
-// typedef JfrArtifactCallbackHost<PkgPtr, PackageWriterWithClear> PackageCallback;
-// typedef JfrArtifactCallbackHost<PkgPtr, CompositePackageWriterWithClear> CompositePackageCallback;
-
-// /*
-//  * Composite operation
-//  *
-//  * LeakpPackageWriter ->
-//  *   PackageWriter ->
-//  *     ClearArtifact<PackageEntry>
-//  *
-//  */
-// void JfrTypeSet::write_package_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer) {
-//   assert(_artifacts->has_klass_entries(), "invariant");
-//   ClearArtifact<PkgPtr> clear(_class_unload);
-//   PackageWriter pw(writer, _artifacts, _class_unload);
-//   if (leakp_writer == NULL) {
-//     PackageWriterWithClear pwwc(&pw, &clear);
-//     KlassPackageWriterWithClear kpwwc(&pwwc);
-//     _artifacts->iterate_klasses(kpwwc);
-//     PackageCallback callback(&pwwc);
-//     _subsystem_callback = &callback;
-//     do_packages();
-//     return;
-//   }
-//   LeakPackageWriter lpw(leakp_writer, _artifacts, _class_unload);
-//   CompositePackageWriter cpw(&lpw, &pw);
-//   CompositePackageWriterWithClear cpwwc(&cpw, &clear);
-//   KlassCompositePackageWriterWithClear ckpw(&cpwwc);
-//   _artifacts->iterate_klasses(ckpw);
-//   CompositePackageCallback callback(&cpwwc);
-//   _subsystem_callback = &callback;
-//   do_packages();
-// }
-
-// typedef CompositeFunctor<ModPtr,
-//                          ModuleWriter,
-//                          ClearArtifact<ModPtr> > ModuleWriterWithClear;
-
-// typedef CompositeFunctor<ModPtr,
-//                          LeakModuleWriter,
-//                          ModuleWriter> CompositeModuleWriter;
-
-// typedef CompositeFunctor<ModPtr,
-//                          CompositeModuleWriter,
-//                          ClearArtifact<ModPtr> > CompositeModuleWriterWithClear;
-
-// typedef JfrArtifactCallbackHost<ModPtr, ModuleWriterWithClear> ModuleCallback;
-// typedef JfrArtifactCallbackHost<ModPtr, CompositeModuleWriterWithClear> CompositeModuleCallback;
+typedef JfrArtifactWriterImplHost<CStringEntryPtr, write__artifact__package> PackageEntryWriterImpl;
+typedef JfrArtifactWriterHost<PackageEntryWriterImpl, TYPE_PACKAGE> PackageEntryWriter;
 
-// XXX
-// class ModuleFieldSelector {
-//  public:
-//   typedef ModPtr TypePtr;
-//   static TypePtr select(KlassPtr klass) {
-//     assert(klass != NULL, "invariant");
-//     PkgPtr pkg = klass->package();
-//     return pkg != NULL ? pkg->module() : NULL;
-//   }
-// };
-
-// typedef KlassToFieldEnvelope<ModuleFieldSelector,
-//                              ModuleWriterWithClear> KlassModuleWriterWithClear;
-
-// typedef KlassToFieldEnvelope<ModuleFieldSelector,
-//                              CompositeModuleWriterWithClear> KlassCompositeModuleWriterWithClear;
+void JfrTypeSet::write_package_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer) {
+  assert(_artifacts->has_klass_entries(), "invariant");
+  assert(writer != NULL, "invariant");
+  // below jdk9 there is no oop for packages, so nothing to do with leakp_writer
+  // just write packages
+  PackageEntryWriter pw(writer, _artifacts, _class_unload);
+  _artifacts->iterate_packages(pw);
+}
 
 typedef CompositeFunctor<CldPtr, CldWriter, ClearArtifact<CldPtr> > CldWriterWithClear;
 typedef CompositeFunctor<CldPtr, LeakCldWriter, CldWriter> CompositeCldWriter;
@@ -892,7 +813,7 @@ void JfrTypeSet::serialize(JfrCheckpointWriter* writer, JfrCheckpointWriter* lea
   // might tag an artifact to be written in a subsequent step
   write_klass_constants(writer, leakp_writer);
   if (_artifacts->has_klass_entries()) {
-// XXX    write_package_constants(writer, leakp_writer);
+    write_package_constants(writer, leakp_writer);
     write_class_loader_constants(writer, leakp_writer);
     write_method_constants(writer, leakp_writer);
     write_symbol_constants(writer, leakp_writer);

src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.hpp

@@ -58,7 +58,7 @@ class JfrTypeSet : AllStatic {
   static void do_class_loaders();
 
   static void write_klass_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer);
-// XXX  static void write_package_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer);
+  static void write_package_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer);
   static void write_class_loader_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer);
   static void write_method_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer);
   static void write_symbol_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer);

src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.cpp

@@ -28,9 +28,11 @@
 #include "oops/oop.inline.hpp"
 #include "oops/symbol.hpp"
 
-JfrSymbolId::JfrSymbolId() : _symbol_id_counter(0), _sym_table(new SymbolTable(this)), _cstring_table(new CStringTable(this)) {
+JfrSymbolId::JfrSymbolId() : _symbol_id_counter(0), _sym_table(new SymbolTable(this)),
+        _cstring_table(new CStringTable(this)), _pkg_table(new CStringTable(this)) {
   assert(_sym_table != NULL, "invariant");
   assert(_cstring_table != NULL, "invariant");
+  assert(_pkg_table != NULL, "invariant");
   initialize();
 }
 
@@ -52,6 +54,11 @@ void JfrSymbolId::clear() {
   }
   assert(!_cstring_table->has_entries(), "invariant");
   _symbol_id_counter = 0;
+  assert(_pkg_table != NULL, "invariant");
+  if (_pkg_table->has_entries()) {
+    _pkg_table->clear_entries();
+  }
+  assert(!_pkg_table->has_entries(), "invariant");
 }
 
 JfrSymbolId::~JfrSymbolId() {
@@ -148,6 +155,12 @@ traceid JfrSymbolId::mark(const char* str, uintptr_t hash) {
   return _cstring_table->id(str, hash);
 }
 
+traceid JfrSymbolId::markPackage(const char* name, uintptr_t hash) {
+  assert(name != NULL, "invariant");
+  assert(_pkg_table != NULL, "invariant");
+  return _pkg_table->id(name, hash);
+}
+
 bool JfrSymbolId::is_anonymous_klass(const Klass* k) {
   assert(k != NULL, "invariant");
   return k->oop_is_instance() && ((const InstanceKlass*)k)->is_anonymous();
@@ -243,6 +256,10 @@ traceid JfrArtifactSet::mark(const char* const str, uintptr_t hash) {
   return _symbol_id->mark(str, hash);
 }
 
+traceid JfrArtifactSet::markPackage(const char* const name, uintptr_t hash) {
+  return _symbol_id->markPackage(name, hash);
+}
+
 const JfrSymbolId::SymbolEntry* JfrArtifactSet::map_symbol(const Symbol* symbol) const {
   return _symbol_id->map_symbol(symbol);
 }

src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp

@@ -230,9 +230,10 @@ class JfrSymbolId : public JfrCHeapObj {
   typedef SymbolTable::HashEntry SymbolEntry;
   typedef CStringTable::HashEntry CStringEntry;
  private:
+  traceid _symbol_id_counter;
   SymbolTable* _sym_table;
   CStringTable* _cstring_table;
-  traceid _symbol_id_counter;
+  CStringTable* _pkg_table;
 
   // hashtable(s) callbacks
   void assign_id(SymbolEntry* entry);
@@ -257,6 +258,12 @@ class JfrSymbolId : public JfrCHeapObj {
   traceid mark(const Klass* k);
   traceid mark(const Symbol* symbol);
   traceid mark(const char* str, uintptr_t hash);
+  traceid markPackage(const char* name, uintptr_t hash);
+
+  template <typename T>
+  void iterate_packages(T& functor) {
+    _pkg_table->iterate_entry(functor);
+  }
 
   const SymbolEntry* map_symbol(const Symbol* symbol) const;
   const SymbolEntry* map_symbol(uintptr_t hash) const;
@@ -334,6 +341,8 @@ class JfrArtifactSet : public JfrCHeapObj {
   traceid mark(const char* const str, uintptr_t hash);
   traceid mark_anonymous_klass_name(const Klass* klass);
 
+  traceid markPackage(const char* const name, uintptr_t hash);
+
   const JfrSymbolId::SymbolEntry* map_symbol(const Symbol* symbol) const;
   const JfrSymbolId::SymbolEntry* map_symbol(uintptr_t hash) const;
   const JfrSymbolId::CStringEntry* map_cstring(uintptr_t hash) const;
@@ -360,6 +369,11 @@ class JfrArtifactSet : public JfrCHeapObj {
   void iterate_cstrings(T& functor) {
     _symbol_id->iterate_cstrings(functor);
   }
+
+  template <typename T>
+  void iterate_packages(T& functor) {
+    _symbol_id->iterate_packages(functor);
+  }
 };
 
 class KlassArtifactRegistrator {