From e6305e1b3f948a82f0d9bd81617c430bf916b016 Mon Sep 17 00:00:00 2001 From: Denghui Dong Date: Sat, 15 Feb 2020 10:27:01 +0800 Subject: [PATCH] [Backport] 8230400: Missing constant pool entry for a method in stacktrace Summary: Test Plan: jdk/jfr Reviewed-by: yuleil Issue: https://github.com/alibaba/dragonwell8/issues/112 --- .../jfrEventClassTransformer.cpp | 48 +------- .../checkpoint/jfrCheckpointManager.cpp | 12 +- .../recorder/checkpoint/types/jfrTypeSet.cpp | 108 ++++++++++++++---- .../checkpoint/types/jfrTypeSetUtils.hpp | 6 - .../types/traceid/jfrTraceIdBits.inline.hpp | 2 +- 5 files changed, 96 insertions(+), 80 deletions(-) diff --git a/src/share/vm/jfr/instrumentation/jfrEventClassTransformer.cpp b/src/share/vm/jfr/instrumentation/jfrEventClassTransformer.cpp index 36945e3ad..57d075b80 100644 --- a/src/share/vm/jfr/instrumentation/jfrEventClassTransformer.cpp +++ b/src/share/vm/jfr/instrumentation/jfrEventClassTransformer.cpp @@ -1437,35 +1437,6 @@ static void rewrite_klass_pointer(InstanceKlass*& ik, InstanceKlass* new_ik, Cla ik = new_ik; } -// During retransform/redefine, copy the Method specific trace flags -// from the previous ik ("the original klass") to the new ik ("the scratch_klass"). -// The open code for retransform/redefine does not know about these. -// In doing this migration here, we ensure the new Methods (defined in scratch klass) -// will carry over trace tags from the old Methods being replaced, -// ensuring flag/tag continuity while being transparent to open code. -static void copy_method_trace_flags(const InstanceKlass* the_original_klass, const InstanceKlass* the_scratch_klass) { - assert(the_original_klass != NULL, "invariant"); - assert(the_scratch_klass != NULL, "invariant"); - assert(the_original_klass->name() == the_scratch_klass->name(), "invariant"); - const Array* old_methods = the_original_klass->methods(); - const Array* new_methods = the_scratch_klass->methods(); - const bool equal_array_length = old_methods->length() == new_methods->length(); - // The Method array has the property of being sorted. - // If they are the same length, there is a one-to-one mapping. - // If they are unequal, there was a method added (currently only - // private static methods allowed to be added), use lookup. - for (int i = 0; i < old_methods->length(); ++i) { - const Method* const old_method = old_methods->at(i); - Method* const new_method = equal_array_length ? new_methods->at(i) : - the_scratch_klass->find_method(old_method->name(), old_method->signature()); - assert(new_method != NULL, "invariant"); - assert(new_method->name() == old_method->name(), "invariant"); - assert(new_method->signature() == old_method->signature(), "invariant"); - new_method->set_trace_flags(old_method->trace_flags()); - assert(new_method->trace_flags() == old_method->trace_flags(), "invariant"); - } -} - static bool is_retransforming(const InstanceKlass* ik, TRAPS) { assert(ik != NULL, "invariant"); assert(JdkJfrEvent::is_a(ik), "invariant"); @@ -1473,16 +1444,7 @@ static bool is_retransforming(const InstanceKlass* ik, TRAPS) { assert(name != NULL, "invariant"); Handle class_loader(THREAD, ik->class_loader()); Handle protection_domain(THREAD, ik->protection_domain()); - // nota bene: use lock-free dictionary lookup - const InstanceKlass* prev_ik = (const InstanceKlass*)SystemDictionary::find(name, class_loader, protection_domain, THREAD); - if (prev_ik == NULL) { - return false; - } - // an existing ik implies a retransform/redefine - assert(prev_ik != NULL, "invariant"); - assert(JdkJfrEvent::is_a(prev_ik), "invariant"); - copy_method_trace_flags(prev_ik, ik); - return true; + return SystemDictionary::find(name, class_loader, protection_domain, THREAD) != NULL; } // target for JFR_ON_KLASS_CREATION hook @@ -1511,12 +1473,8 @@ void JfrEventClassTransformer::on_klass_creation(InstanceKlass*& ik, ClassFilePa return; } assert(JdkJfrEvent::is_subklass(ik), "invariant"); - if (is_retransforming(ik, THREAD)) { - // not the initial klass load - return; - } - if (ik->is_abstract()) { - // abstract classes are not instrumented + if (ik->is_abstract() || is_retransforming(ik, THREAD)) { + // abstract and scratch classes are not instrumented return; } ResourceMark rm(THREAD); diff --git a/src/share/vm/jfr/recorder/checkpoint/jfrCheckpointManager.cpp b/src/share/vm/jfr/recorder/checkpoint/jfrCheckpointManager.cpp index f1457a9d3..c28dfb81b 100644 --- a/src/share/vm/jfr/recorder/checkpoint/jfrCheckpointManager.cpp +++ b/src/share/vm/jfr/recorder/checkpoint/jfrCheckpointManager.cpp @@ -365,13 +365,13 @@ void JfrCheckpointManager::write_type_set() { if (!LeakProfiler::is_running()) { JfrCheckpointWriter writer(true, true, Thread::current()); JfrTypeSet::serialize(&writer, NULL, false); - return; + } else { + Thread* const t = Thread::current(); + JfrCheckpointWriter leakp_writer(false, true, t); + JfrCheckpointWriter writer(false, true, t); + JfrTypeSet::serialize(&writer, &leakp_writer, false); + ObjectSampleCheckpoint::on_type_set(leakp_writer); } - Thread* const t = Thread::current(); - JfrCheckpointWriter leakp_writer(false, true, t); - JfrCheckpointWriter writer(false, true, t); - JfrTypeSet::serialize(&writer, &leakp_writer, false); - ObjectSampleCheckpoint::on_type_set(leakp_writer); } void JfrCheckpointManager::write_type_set_for_unloaded_classes() { diff --git a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp index 6eff9c222..ac88cda05 100644 --- a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp +++ b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp @@ -42,6 +42,7 @@ #include "oops/objArrayKlass.hpp" #include "oops/oop.inline.hpp" #include "utilities/accessFlags.hpp" +#include "utilities/bitMap.inline.hpp" typedef const Klass* KlassPtr; // XXX typedef const PackageEntry* PkgPtr; @@ -125,9 +126,7 @@ static traceid method_id(KlassPtr klass, MethodPtr method) { } static traceid cld_id(CldPtr cld, bool leakp) { assert(cld != NULL, "invariant"); - if (cld->is_anonymous()) { - return 0; - } + assert(!cld->is_anonymous(), "invariant"); if (leakp) { SET_LEAKP(cld); } else { @@ -142,6 +141,17 @@ static s4 get_flags(const T* ptr) { return ptr->access_flags().get_flags(); } +static bool is_unsafe_anonymous(const Klass* klass) { + assert(klass != NULL, "invariant"); + return klass->oop_is_instance() && ((const InstanceKlass*)klass)->is_anonymous(); +} + +static ClassLoaderData* get_cld(const Klass* klass) { + assert(klass != NULL, "invariant"); + return is_unsafe_anonymous(klass) ? + InstanceKlass::cast((Klass*)klass)->host_klass()->class_loader_data() : klass->class_loader_data(); +} + template static void set_serialized(const T* ptr) { assert(ptr != NULL, "invariant"); @@ -173,7 +183,7 @@ static int write_klass(JfrCheckpointWriter* writer, KlassPtr klass, bool leakp) assert(theklass->oop_is_typeArray(), "invariant"); } writer->write(artifact_id(klass)); - writer->write(cld_id(klass->class_loader_data(), leakp)); + writer->write(cld_id(get_cld(klass), leakp)); writer->write(mark_symbol(klass, leakp)); writer->write(pkg_id); writer->write(get_flags(klass)); @@ -381,13 +391,22 @@ static void do_class_loader_data(ClassLoaderData* cld) { do_previous_epoch_artifact(_subsystem_callback, cld); } -class CldFieldSelector { +class KlassCldFieldSelector { public: typedef CldPtr TypePtr; static TypePtr select(KlassPtr klass) { assert(klass != NULL, "invariant"); - CldPtr cld = klass->class_loader_data(); - return cld->is_anonymous() ? NULL : cld; + return get_cld(klass); + } +}; + +class ModuleCldFieldSelector { +public: + typedef CldPtr TypePtr; + static TypePtr select(KlassPtr klass) { + assert(klass != NULL, "invariant"); + // ModPtr mod = ModuleFieldSelector::select(klass); + return NULL; // return mod != NULL ? mod->loader_data() : NULL; } }; @@ -413,14 +432,18 @@ typedef JfrPredicatedTypeWriterImplHost CldWriter; typedef CompositeFunctor > CldWriterWithClear; typedef JfrArtifactCallbackHost CldCallback; -typedef KlassToFieldEnvelope KlassCldWriter; +typedef KlassToFieldEnvelope KlassCldWriter; +typedef KlassToFieldEnvelope ModuleCldWriter; +typedef CompositeFunctor KlassAndModuleCldWriter; typedef LeakPredicate LeakCldPredicate; typedef JfrPredicatedTypeWriterImplHost LeakCldWriterImpl; typedef JfrTypeWriterHost LeakCldWriter; typedef CompositeFunctor CompositeCldWriter; -typedef KlassToFieldEnvelope KlassCompositeCldWriter; +typedef KlassToFieldEnvelope KlassCompositeCldWriter; +typedef KlassToFieldEnvelope ModuleCompositeCldWriter; +typedef CompositeFunctor KlassAndModuleCompositeCldWriter; typedef CompositeFunctor > CompositeCldWriterWithClear; typedef JfrArtifactCallbackHost CompositeCldCallback; @@ -428,14 +451,16 @@ static void write_classloaders() { assert(_writer != NULL, "invariant"); CldWriter cldw(_writer, _class_unload); KlassCldWriter kcw(&cldw); + ModuleCldWriter mcw(&cldw); + KlassAndModuleCldWriter kmcw(&kcw, &mcw); if (current_epoch()) { - _artifacts->iterate_klasses(kcw); + _artifacts->iterate_klasses(kmcw); _artifacts->tally(cldw); return; } assert(previous_epoch(), "invariant"); if (_leakp_writer == NULL) { - _artifacts->iterate_klasses(kcw); + _artifacts->iterate_klasses(kmcw); ClearArtifact clear; CldWriterWithClear cldwwc(&cldw, &clear); CldCallback callback(&cldwwc); @@ -445,7 +470,9 @@ static void write_classloaders() { LeakCldWriter lcldw(_leakp_writer, _class_unload); CompositeCldWriter ccldw(&lcldw, &cldw); KlassCompositeCldWriter kccldw(&ccldw); - _artifacts->iterate_klasses(kccldw); + ModuleCompositeCldWriter mccldw(&ccldw); + KlassAndModuleCompositeCldWriter kmccldw(&kccldw, &mccldw); + _artifacts->iterate_klasses(kmccldw); ClearArtifact clear; CompositeCldWriterWithClear ccldwwc(&ccldw, &clear); CompositeCldCallback callback(&ccldwwc); @@ -495,7 +522,31 @@ int write__method__leakp(JfrCheckpointWriter* writer, const void* m) { return write_method(writer, method, true); } -template +class BitMapFilter { + BitMap _bitmap; + public: + explicit BitMapFilter(int length = 0) : _bitmap((size_t)length) {} + bool operator()(size_t idx) { + if (_bitmap.size() == 0) { + return true; + } + if (_bitmap.at(idx)) { + return false; + } + _bitmap.set_bit(idx); + return true; + } +}; + +class AlwaysTrue { + public: + explicit AlwaysTrue(int length = 0) {} + bool operator()(size_t idx) { + return true; + } +}; + +template class MethodIteratorHost { private: MethodCallback _method_cb; @@ -514,13 +565,19 @@ class MethodIteratorHost { bool operator()(KlassPtr klass) { if (_method_used_predicate(klass)) { - const InstanceKlass* const ik = InstanceKlass::cast((Klass*)klass); + const InstanceKlass* ik = InstanceKlass::cast((Klass*)klass); const int len = ik->methods()->length(); - for (int i = 0; i < len; ++i) { - MethodPtr method = ik->methods()->at(i); - if (_method_flag_predicate(method)) { - _method_cb(method); + Filter filter(ik->previous_versions() != NULL ? len : 0); + while (ik != NULL) { + for (int i = 0; i < len; ++i) { + MethodPtr method = ik->methods()->at(i); + if (_method_flag_predicate(method) && filter(i)) { + _method_cb(method); + } } + // There can be multiple versions of the same method running + // due to redefinition. Need to inspect the complete set of methods. + ik = ik->previous_versions(); } } return _klass_cb(klass); @@ -540,16 +597,23 @@ class Wrapper { } }; +template +class EmptyStub { + public: + bool operator()(T const& value) { return true; } +}; + typedef SerializePredicate MethodPredicate; typedef JfrPredicatedTypeWriterImplHost MethodWriterImplTarget; -typedef Wrapper KlassCallbackStub; +typedef Wrapper KlassCallbackStub; typedef JfrTypeWriterHost MethodWriterImpl; -typedef MethodIteratorHost MethodWriter; +typedef MethodIteratorHost MethodWriter; typedef LeakPredicate LeakMethodPredicate; typedef JfrPredicatedTypeWriterImplHost LeakMethodWriterImplTarget; typedef JfrTypeWriterHost LeakMethodWriterImpl; -typedef MethodIteratorHost LeakMethodWriter; +typedef MethodIteratorHost LeakMethodWriter; +typedef MethodIteratorHost LeakMethodWriter; typedef CompositeFunctor CompositeMethodWriter; static void write_methods() { @@ -675,7 +739,7 @@ void JfrTypeSet::clear() { typedef Wrapper ClearKlassBits; typedef Wrapper ClearMethodFlag; -typedef MethodIteratorHost ClearKlassAndMethods; +typedef MethodIteratorHost ClearKlassAndMethods; static size_t teardown() { assert(_artifacts != NULL, "invariant"); diff --git a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp index 5634ccc9d..44fe244d6 100644 --- a/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp +++ b/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp @@ -103,12 +103,6 @@ class ClearArtifact { } }; -template -class JfrStub { - public: - bool operator()(T const& value) { return true; } -}; - template class SerializePredicate { bool _class_unload; diff --git a/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp b/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp index 490f51e1c..fbe773d20 100644 --- a/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp +++ b/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp @@ -37,7 +37,7 @@ static const int low_offset = 7; static const int meta_offset = low_offset - 1; #endif -inline void set_bits(jbyte bits, jbyte* const dest) { +inline void set_bits(jbyte bits, jbyte volatile* const dest) { assert(dest != NULL, "invariant"); if (bits != (*dest & bits)) { *dest |= bits; -- GitLab