# HG changeset patch # User apetushkov # Date 1592219291 -10800 # Node ID d20a5f399218f58f82f4f4503d24957ce7e48e60 # Parent 30fb8c8cceb96360b96aa48a5b31f4a1d29bd36d 8245167: Top package in method profiling shows null in JMC Reviewed-by: neugens Contributed-by: asemenov@azul.com diff -r 30fb8c8cceb9 -r d20a5f399218 src/share/vm/classfile/classLoader.cpp --- a/src/share/vm/classfile/classLoader.cpp Fri Jun 12 02:59:56 2020 +0100 +++ b/src/share/vm/classfile/classLoader.cpp Mon Jun 15 14:08:11 2020 +0300 @@ -162,6 +162,64 @@ 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) { diff -r 30fb8c8cceb9 -r d20a5f399218 src/share/vm/classfile/classLoader.hpp --- a/src/share/vm/classfile/classLoader.hpp Fri Jun 12 02:59:56 2020 +0100 +++ b/src/share/vm/classfile/classLoader.hpp Mon Jun 15 14:08:11 2020 +0300 @@ -366,6 +366,11 @@ // 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; diff -r 30fb8c8cceb9 -r d20a5f399218 src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp --- a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp Fri Jun 12 02:59:56 2020 +0100 +++ b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp Mon Jun 15 14:08:11 2020 +0300 @@ -52,6 +52,7 @@ // 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 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 @@ 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 @@ typedef JfrArtifactWriterImplHost MethodWriterImplTarget; typedef JfrArtifactWriterHost 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; -// } +int write__artifact__package(JfrCheckpointWriter* writer, JfrArtifactSet* artifacts, const void* p) { + assert(writer != NULL, "invariant"); + assert(artifacts != NULL, "invariant"); + assert(p != NULL, "invariant"); -// typedef LeakPredicate LeakPackagePredicate; -// int _compare_pkg_ptr_(PkgPtr const& lhs, PkgPtr const& rhs) { return lhs > rhs ? 1 : (lhs < rhs) ? -1 : 0; } -// typedef UniquePredicate PackagePredicate; -// typedef JfrPredicatedArtifactWriterImplHost LeakPackageWriterImpl; -// typedef JfrPredicatedArtifactWriterImplHost PackageWriterImpl; -// typedef JfrArtifactWriterHost LeakPackageWriter; -// typedef JfrArtifactWriterHost PackageWriter; + 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 @@ do_klasses(); } -// XXX -// typedef CompositeFunctor > PackageWriterWithClear; +typedef JfrArtifactWriterImplHost PackageEntryWriterImpl; +typedef JfrArtifactWriterHost PackageEntryWriter; -// typedef CompositeFunctor CompositePackageWriter; - -// typedef CompositeFunctor > CompositePackageWriterWithClear; - -// class PackageFieldSelector { -// public: -// typedef PkgPtr TypePtr; -// static TypePtr select(KlassPtr klass) { -// assert(klass != NULL, "invariant"); -// return ((InstanceKlass*)klass)->package(); -// } -// }; - -// typedef KlassToFieldEnvelope KlassPackageWriterWithClear; - -// typedef KlassToFieldEnvelope KlassCompositePackageWriterWithClear; - -// typedef JfrArtifactCallbackHost PackageCallback; -// typedef JfrArtifactCallbackHost CompositePackageCallback; - -// /* -// * Composite operation -// * -// * LeakpPackageWriter -> -// * PackageWriter -> -// * ClearArtifact -// * -// */ -// void JfrTypeSet::write_package_constants(JfrCheckpointWriter* writer, JfrCheckpointWriter* leakp_writer) { -// assert(_artifacts->has_klass_entries(), "invariant"); -// ClearArtifact 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 > ModuleWriterWithClear; - -// typedef CompositeFunctor CompositeModuleWriter; - -// typedef CompositeFunctor > CompositeModuleWriterWithClear; - -// typedef JfrArtifactCallbackHost ModuleCallback; -// typedef JfrArtifactCallbackHost CompositeModuleCallback; - -// 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 KlassModuleWriterWithClear; - -// typedef KlassToFieldEnvelope 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 > CldWriterWithClear; typedef CompositeFunctor CompositeCldWriter; @@ -892,7 +813,7 @@ // 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); diff -r 30fb8c8cceb9 -r d20a5f399218 src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.hpp --- a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.hpp Fri Jun 12 02:59:56 2020 +0100 +++ b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.hpp Mon Jun 15 14:08:11 2020 +0300 @@ -58,7 +58,7 @@ 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); diff -r 30fb8c8cceb9 -r d20a5f399218 src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.cpp --- a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.cpp Fri Jun 12 02:59:56 2020 +0100 +++ b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.cpp Mon Jun 15 14:08:11 2020 +0300 @@ -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 @@ } 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 @@ 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 @@ 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); } diff -r 30fb8c8cceb9 -r d20a5f399218 src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp --- a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp Fri Jun 12 02:59:56 2020 +0100 +++ b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp Mon Jun 15 14:08:11 2020 +0300 @@ -230,9 +230,10 @@ 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 @@ 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 + 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 @@ 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 @@ void iterate_cstrings(T& functor) { _symbol_id->iterate_cstrings(functor); } + + template + void iterate_packages(T& functor) { + _symbol_id->iterate_packages(functor); + } }; class KlassArtifactRegistrator {