diff options
-rw-r--r-- | runtime/jdwp/jdwp_event.cc | 47 | ||||
-rw-r--r-- | runtime/reference_table.cc | 96 | ||||
-rw-r--r-- | runtime/reference_table_test.cc | 73 |
3 files changed, 173 insertions, 43 deletions
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc index 06b67b3e4d..0d862fe672 100644 --- a/runtime/jdwp/jdwp_event.cc +++ b/runtime/jdwp/jdwp_event.cc @@ -621,8 +621,8 @@ void JdwpState::SendRequestAndPossiblySuspend(ExpandBuf* pReq, JdwpSuspendPolicy Thread* const self = Thread::Current(); self->AssertThreadSuspensionIsAllowable(); CHECK(pReq != nullptr); + CHECK_EQ(threadId, Dbg::GetThreadSelfId()) << "Only the current thread can suspend itself"; /* send request and possibly suspend ourselves */ - JDWP::ObjectId thread_self_id = Dbg::GetThreadSelfId(); ScopedThreadSuspension sts(self, kWaitingForDebuggerSend); if (suspend_policy != SP_NONE) { AcquireJdwpTokenForEvent(threadId); @@ -631,7 +631,7 @@ void JdwpState::SendRequestAndPossiblySuspend(ExpandBuf* pReq, JdwpSuspendPolicy { // Before suspending, we change our state to kSuspended so the debugger sees us as RUNNING. ScopedThreadStateChange stsc(self, kSuspended); - SuspendByPolicy(suspend_policy, thread_self_id); + SuspendByPolicy(suspend_policy, threadId); } } @@ -658,13 +658,10 @@ void JdwpState::ReleaseJdwpTokenForCommand() { } void JdwpState::AcquireJdwpTokenForEvent(ObjectId threadId) { - CHECK_NE(Thread::Current(), GetDebugThread()) << "Expected event thread"; - CHECK_NE(debug_thread_id_, threadId) << "Not expected debug thread"; SetWaitForJdwpToken(threadId); } void JdwpState::ReleaseJdwpTokenForEvent() { - CHECK_NE(Thread::Current(), GetDebugThread()) << "Expected event thread"; ClearWaitForJdwpToken(); } @@ -685,23 +682,28 @@ void JdwpState::SetWaitForJdwpToken(ObjectId threadId) { /* this is held for very brief periods; contention is unlikely */ MutexLock mu(self, jdwp_token_lock_); - CHECK_NE(jdwp_token_owner_thread_id_, threadId) << "Thread is already holding event thread lock"; + if (jdwp_token_owner_thread_id_ == threadId) { + // Only the debugger thread may already hold the event token. For instance, it may trigger + // a CLASS_PREPARE event while processing a command that initializes a class. + CHECK_EQ(threadId, debug_thread_id_) << "Non-debugger thread is already holding event token"; + } else { + /* + * If another thread is already doing stuff, wait for it. This can + * go to sleep indefinitely. + */ - /* - * If another thread is already doing stuff, wait for it. This can - * go to sleep indefinitely. - */ - while (jdwp_token_owner_thread_id_ != 0) { - VLOG(jdwp) << StringPrintf("event in progress (%#" PRIx64 "), %#" PRIx64 " sleeping", - jdwp_token_owner_thread_id_, threadId); - waited = true; - jdwp_token_cond_.Wait(self); - } + while (jdwp_token_owner_thread_id_ != 0) { + VLOG(jdwp) << StringPrintf("event in progress (%#" PRIx64 "), %#" PRIx64 " sleeping", + jdwp_token_owner_thread_id_, threadId); + waited = true; + jdwp_token_cond_.Wait(self); + } - if (waited || threadId != debug_thread_id_) { - VLOG(jdwp) << StringPrintf("event token grabbed (%#" PRIx64 ")", threadId); + if (waited || threadId != debug_thread_id_) { + VLOG(jdwp) << StringPrintf("event token grabbed (%#" PRIx64 ")", threadId); + } + jdwp_token_owner_thread_id_ = threadId; } - jdwp_token_owner_thread_id_ = threadId; } /* @@ -1224,14 +1226,15 @@ void JdwpState::PostClassPrepare(mirror::Class* klass) { VLOG(jdwp) << " suspend_policy=" << suspend_policy; } - if (thread_id == debug_thread_id_) { + ObjectId reported_thread_id = thread_id; + if (reported_thread_id == debug_thread_id_) { /* * JDWP says that, for a class prep in the debugger thread, we * should set thread to null and if any threads were supposed * to be suspended then we suspend all other threads. */ VLOG(jdwp) << " NOTE: class prepare in debugger thread!"; - thread_id = 0; + reported_thread_id = 0; if (suspend_policy == SP_EVENT_THREAD) { suspend_policy = SP_ALL; } @@ -1244,7 +1247,7 @@ void JdwpState::PostClassPrepare(mirror::Class* klass) { for (const JdwpEvent* pEvent : match_list) { expandBufAdd1(pReq, pEvent->eventKind); expandBufAdd4BE(pReq, pEvent->requestId); - expandBufAddObjectId(pReq, thread_id); + expandBufAddObjectId(pReq, reported_thread_id); expandBufAdd1(pReq, tag); expandBufAddRefTypeId(pReq, class_id); expandBufAddUtf8String(pReq, signature); diff --git a/runtime/reference_table.cc b/runtime/reference_table.cc index d4bd6dd247..62e23ed92b 100644 --- a/runtime/reference_table.cc +++ b/runtime/reference_table.cc @@ -215,33 +215,87 @@ void ReferenceTable::Dump(std::ostream& os, Table& entries) { } std::sort(sorted_entries.begin(), sorted_entries.end(), GcRootComparator()); - // Dump a summary of the whole table. - os << " Summary:\n"; - size_t equiv = 0; - size_t identical = 0; - mirror::Object* prev = nullptr; - for (GcRoot<mirror::Object>& root : sorted_entries) { - mirror::Object* current = root.Read<kWithoutReadBarrier>(); - if (prev != nullptr) { - const size_t element_count = GetElementCount(prev); - if (current == prev) { + class SummaryElement { + public: + GcRoot<mirror::Object> root; + size_t equiv; + size_t identical; + + SummaryElement() : equiv(0), identical(0) {} + SummaryElement(SummaryElement&& ref) { + root = ref.root; + equiv = ref.equiv; + identical = ref.identical; + } + SummaryElement(const SummaryElement&) = default; + SummaryElement& operator=(SummaryElement&&) = default; + + void Reset(GcRoot<mirror::Object>& _root) { + root = _root; + equiv = 0; + identical = 0; + } + }; + std::vector<SummaryElement> sorted_summaries; + { + SummaryElement prev; + + for (GcRoot<mirror::Object>& root : sorted_entries) { + mirror::Object* current = root.Read<kWithoutReadBarrier>(); + + if (UNLIKELY(prev.root.IsNull())) { + prev.Reset(root); + continue; + } + + mirror::Object* prevObj = prev.root.Read<kWithoutReadBarrier>(); + if (current == prevObj) { // Same reference, added more than once. - ++identical; - } else if (current->GetClass() == prev->GetClass() && - GetElementCount(current) == element_count) { + ++prev.identical; + } else if (current->GetClass() == prevObj->GetClass() && + GetElementCount(current) == GetElementCount(prevObj)) { // Same class / element count, different object. - ++equiv; + ++prev.equiv; } else { - // Different class. - DumpSummaryLine(os, prev, element_count, identical, equiv); - equiv = 0; - identical = 0; + sorted_summaries.push_back(prev); + prev.Reset(root); } + prev.root = root; } - prev = current; + sorted_summaries.push_back(prev); + + // Compare summary elements, first by combined count, then by identical (indicating leaks), + // then by class (and size and address). + struct SummaryElementComparator { + GcRootComparator gc_root_cmp; + + bool operator()(SummaryElement& elem1, SummaryElement& elem2) const + NO_THREAD_SAFETY_ANALYSIS { + Locks::mutator_lock_->AssertSharedHeld(Thread::Current()); + + size_t count1 = elem1.equiv + elem1.identical; + size_t count2 = elem2.equiv + elem2.identical; + if (count1 != count2) { + return count1 > count2; + } + + if (elem1.identical != elem2.identical) { + return elem1.identical > elem2.identical; + } + + // Otherwise, compare the GC roots as before. + return gc_root_cmp(elem1.root, elem2.root); + } + }; + std::sort(sorted_summaries.begin(), sorted_summaries.end(), SummaryElementComparator()); + } + + // Dump a summary of the whole table. + os << " Summary:\n"; + for (SummaryElement& elem : sorted_summaries) { + mirror::Object* elemObj = elem.root.Read<kWithoutReadBarrier>(); + DumpSummaryLine(os, elemObj, GetElementCount(elemObj), elem.identical, elem.equiv); } - // Handle the last entry. - DumpSummaryLine(os, prev, GetElementCount(prev), identical, equiv); } void ReferenceTable::VisitRoots(RootVisitor* visitor, const RootInfo& root_info) { diff --git a/runtime/reference_table_test.cc b/runtime/reference_table_test.cc index fae8e722c3..c48edbe0cf 100644 --- a/runtime/reference_table_test.cc +++ b/runtime/reference_table_test.cc @@ -106,4 +106,77 @@ TEST_F(ReferenceTableTest, Basics) { } } +static std::vector<size_t> FindAll(const std::string& haystack, const char* needle) { + std::vector<size_t> res; + size_t start = 0; + do { + size_t pos = haystack.find(needle, start); + if (pos == std::string::npos) { + break; + } + res.push_back(pos); + start = pos + 1; + } while (start < haystack.size()); + return res; +} + +TEST_F(ReferenceTableTest, SummaryOrder) { + // Check that the summary statistics are sorted. + ScopedObjectAccess soa(Thread::Current()); + + ReferenceTable rt("test", 0, 20); + + { + mirror::Object* s1 = mirror::String::AllocFromModifiedUtf8(soa.Self(), "hello"); + mirror::Object* s2 = mirror::String::AllocFromModifiedUtf8(soa.Self(), "world"); + + // 3 copies of s1, 2 copies of s2, interleaved. + for (size_t i = 0; i != 2; ++i) { + rt.Add(s1); + rt.Add(s2); + } + rt.Add(s1); + } + + { + // Differently sized byte arrays. Should be sorted by identical (non-unique cound). + mirror::Object* b1_1 = mirror::ByteArray::Alloc(soa.Self(), 1); + rt.Add(b1_1); + rt.Add(mirror::ByteArray::Alloc(soa.Self(), 2)); + rt.Add(b1_1); + rt.Add(mirror::ByteArray::Alloc(soa.Self(), 2)); + rt.Add(mirror::ByteArray::Alloc(soa.Self(), 1)); + rt.Add(mirror::ByteArray::Alloc(soa.Self(), 2)); + } + + rt.Add(mirror::CharArray::Alloc(soa.Self(), 0)); + + // Now dump, and ensure order. + std::ostringstream oss; + rt.Dump(oss); + + // Only do this on the part after Summary. + std::string base = oss.str(); + size_t summary_pos = base.find("Summary:"); + ASSERT_NE(summary_pos, std::string::npos); + + std::string haystack = base.substr(summary_pos); + + std::vector<size_t> strCounts = FindAll(haystack, "java.lang.String"); + std::vector<size_t> b1Counts = FindAll(haystack, "byte[] (1 elements)"); + std::vector<size_t> b2Counts = FindAll(haystack, "byte[] (2 elements)"); + std::vector<size_t> cCounts = FindAll(haystack, "char[]"); + + // Only one each. + EXPECT_EQ(1u, strCounts.size()); + EXPECT_EQ(1u, b1Counts.size()); + EXPECT_EQ(1u, b2Counts.size()); + EXPECT_EQ(1u, cCounts.size()); + + // Expect them to be in order. + EXPECT_LT(strCounts[0], b1Counts[0]); + EXPECT_LT(b1Counts[0], b2Counts[0]); + EXPECT_LT(b2Counts[0], cCounts[0]); +} + } // namespace art |