From b33635d2f9f7ffe10ba94dd1cc695897350d3e5e Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Mon, 14 Nov 2016 17:00:28 -0800 Subject: ART: Prioritize reference table dump Sort the reference table summary dump by highest count, prioritizing repeated types and instances. This will help with triaging leaks. Add a test. (cherry picked from commit 6e970e7fa88efd5ee38b0d6f9010a3985c62778f) Bug: 32857749 Test: m test-art-host-gtest-reference_table_test Merged-In: Id9c7a118c36c06c7510b13c2a51e63f08a27c564 Change-Id: I7e61881b5badf9ac2b6b72333f31437ab498caee --- runtime/reference_table.cc | 96 ++++++++++++++++++++++++++++++++--------- runtime/reference_table_test.cc | 73 +++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 21 deletions(-) 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& root : sorted_entries) { - mirror::Object* current = root.Read(); - if (prev != nullptr) { - const size_t element_count = GetElementCount(prev); - if (current == prev) { + class SummaryElement { + public: + GcRoot 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& _root) { + root = _root; + equiv = 0; + identical = 0; + } + }; + std::vector sorted_summaries; + { + SummaryElement prev; + + for (GcRoot& root : sorted_entries) { + mirror::Object* current = root.Read(); + + if (UNLIKELY(prev.root.IsNull())) { + prev.Reset(root); + continue; + } + + mirror::Object* prevObj = prev.root.Read(); + 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(); + 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 FindAll(const std::string& haystack, const char* needle) { + std::vector 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 strCounts = FindAll(haystack, "java.lang.String"); + std::vector b1Counts = FindAll(haystack, "byte[] (1 elements)"); + std::vector b2Counts = FindAll(haystack, "byte[] (2 elements)"); + std::vector 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 -- cgit v1.2.3 From 2b25ca97ff20ab189567391e861dcd14204a1646 Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Tue, 22 Nov 2016 14:55:04 +0100 Subject: Fix event reporting from the debugger thread The debugger thread may trigger events (like CLASS PREPARE after the initalization of a class while processing a command). This CL removes the incorrect CHECK that makes the runtime abort in this case. However, we do check that only the debugger thread can report an event while it is already processing a command. Bug: 33032664 Test: art/tools/run-jdwp-tests.sh '--mode=host' '--variant=X64' (cherry picked from commit af8bcf83535cd7bf5651ada2fb08a0ba9c5191d6) Change-Id: Id2e4362393203935e820586560800b300c6dd3a3 --- runtime/jdwp/jdwp_event.cc | 47 ++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 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); -- cgit v1.2.3 From 8394cee5f0f87ddeeb8f33c199fa21e0cdee9073 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Fri, 5 Aug 2016 16:09:09 -0700 Subject: Prune uses library classes even without profile DO NOT MERGE The previous pruning relied on the classes being pruned from the profile, and then using the profile to prune classes. If there was no profile, the uses library classes were incorrectly left unpruned. Leaving these classes unpruned caused aborts during compilation. Cherry picked from https://android-review.googlesource.com/#/c/312750/ Bug: 30688277 Bug: 31427727 Test: adb shell dex2oat --runtime-arg -classpath --runtime-arg /system/framework/com.google.android.maps.jar --dex-file=/data/app/comb.BBClient-1/base.apk --dex-location=/data/app/comb.BBClient-1/base.apk --oat-file=/data/app/comb.BBClient-1/oat/arm/base.odex --app-image-file=/data/app/comb.BBClient-1/oat/arm/base.art --image-format=lz4 --compiler-filter=speed Change-Id: I261b8894847b5b0a4f7330f49666e823a1b38bb0 --- compiler/image_writer.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 61cf00942e..2d6c4dab6a 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -789,6 +789,13 @@ bool ImageWriter::PruneAppImageClassInternal( result = result || PruneAppImageClassInternal(klass->GetSuperClass(), &my_early_exit, visited); + // Remove the class if the dex file is not in the set of dex files. This happens for classes that + // are from uses library if there is no profile. b/30688277 + mirror::DexCache* dex_cache = klass->GetDexCache(); + if (dex_cache != nullptr) { + result = result || + dex_file_oat_index_map_.find(dex_cache->GetDexFile()) == dex_file_oat_index_map_.end(); + } // Erase the element we stored earlier since we are exiting the function. auto it = visited->find(klass); DCHECK(it != visited->end()); -- cgit v1.2.3 From dfd17f18c9e91cc6bf3098c40347c39d14c32412 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Fri, 5 Aug 2016 16:09:09 -0700 Subject: Prune uses library classes even without profile The previous pruning relied on the classes being pruned from the profile, and then using the profile to prune classes. If there was no profile, the uses library classes were incorrectly left unpruned. Leaving these classes unpruned caused aborts during compilation. Bug: 30688277 Test: adb shell dex2oat --runtime-arg -classpath --runtime-arg /system/framework/com.google.android.maps.jar --dex-file=/data/app/comb.BBClient-1/base.apk --dex-location=/data/app/comb.BBClient-1/base.apk --oat-file=/data/app/comb.BBClient-1/oat/arm/base.odex --app-image-file=/data/app/comb.BBClient-1/oat/arm/base.art --image-format=lz4 --compiler-filter=speed (cherry picked from commit c27bc405312845bda4e25cd24e93211dfc7d7308) Change-Id: I261b8894847b5b0a4f7330f49666e823a1b38bb0 --- compiler/image_writer.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 61cf00942e..2d6c4dab6a 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -789,6 +789,13 @@ bool ImageWriter::PruneAppImageClassInternal( result = result || PruneAppImageClassInternal(klass->GetSuperClass(), &my_early_exit, visited); + // Remove the class if the dex file is not in the set of dex files. This happens for classes that + // are from uses library if there is no profile. b/30688277 + mirror::DexCache* dex_cache = klass->GetDexCache(); + if (dex_cache != nullptr) { + result = result || + dex_file_oat_index_map_.find(dex_cache->GetDexFile()) == dex_file_oat_index_map_.end(); + } // Erase the element we stored earlier since we are exiting the function. auto it = visited->find(klass); DCHECK(it != visited->end()); -- cgit v1.2.3 From fc9bec20893d50f9c92c7c25845f2f425f4de052 Mon Sep 17 00:00:00 2001 From: Griff Hazen Date: Mon, 19 Dec 2016 18:15:53 +0000 Subject: Revert "Prune uses library classes even without profile DO NOT MERGE" This fix may be causing other crashes e.g. b/33718412. Bug: 33718412 Bug: 30688277 Bug: 31427727 This reverts commit 8394cee5f0f87ddeeb8f33c199fa21e0cdee9073. Change-Id: Ie19ece9d8ae1afef8b578482fae1cdbd35ff930d --- compiler/image_writer.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 2d6c4dab6a..61cf00942e 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -789,13 +789,6 @@ bool ImageWriter::PruneAppImageClassInternal( result = result || PruneAppImageClassInternal(klass->GetSuperClass(), &my_early_exit, visited); - // Remove the class if the dex file is not in the set of dex files. This happens for classes that - // are from uses library if there is no profile. b/30688277 - mirror::DexCache* dex_cache = klass->GetDexCache(); - if (dex_cache != nullptr) { - result = result || - dex_file_oat_index_map_.find(dex_cache->GetDexFile()) == dex_file_oat_index_map_.end(); - } // Erase the element we stored earlier since we are exiting the function. auto it = visited->find(klass); DCHECK(it != visited->end()); -- cgit v1.2.3 From 9d5d85ecab4e7900e28cfd363897c7086dbda6d4 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Thu, 5 Jan 2017 10:58:45 -0800 Subject: Revert "Revert "Prune uses library classes even without profile DO NOT MERGE"" No evidence that the fix was causing crashes. Bug: 33718412 Bug: 30688277 Bug: 31427727 This reverts commit fc9bec20893d50f9c92c7c25845f2f425f4de052. Test: test-art-host Change-Id: Ib05e9a3ac6079ac246cb2ff00ea133aeddf5cbf3 --- compiler/image_writer.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 61cf00942e..2d6c4dab6a 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -789,6 +789,13 @@ bool ImageWriter::PruneAppImageClassInternal( result = result || PruneAppImageClassInternal(klass->GetSuperClass(), &my_early_exit, visited); + // Remove the class if the dex file is not in the set of dex files. This happens for classes that + // are from uses library if there is no profile. b/30688277 + mirror::DexCache* dex_cache = klass->GetDexCache(); + if (dex_cache != nullptr) { + result = result || + dex_file_oat_index_map_.find(dex_cache->GetDexFile()) == dex_file_oat_index_map_.end(); + } // Erase the element we stored earlier since we are exiting the function. auto it = visited->find(klass); DCHECK(it != visited->end()); -- cgit v1.2.3 From 40d4c7636e51f910b2c9ef226b7183e6ccc9ab4b Mon Sep 17 00:00:00 2001 From: "neo.chae" Date: Tue, 8 Nov 2016 08:40:46 +0900 Subject: Add visiting for class loaders in StickyMarkSweep StickyMarkSweep clear the mark stack, Because all reachable objects must be referenced by a root or a dirty card. But, there are some marking hole for class object. If some object is marked and the object and it's class object is not dirty, Then class object cannot be marking by card table. In previous OS including mashmellow, Class table was maintaned by class linker and all class object was marked with kVisitRootFlagAllRoots flag. In N OS, Class object is not marked with kVisitRootFlagAllRoots. So, I added new flag to mark class object and using it StickyMarkSweep. Added regression test in 141-class-unload. Test: test-art-host Merged-In: I57599e6db53b260f4c5ef466b63962141b8da5c3 (cherry picked from commit a2d1b28599e38ee0180f0f7130a879eac5be9dec) Bug: 33924225 Change-Id: I57599e6db53b260f4c5ef466b63962141b8da5c3 Signed-off-by: Hyangseok Chae --- runtime/class_linker.cc | 2 +- runtime/gc/collector/mark_sweep.cc | 3 +-- runtime/gc/collector/mark_sweep.h | 2 +- runtime/gc/collector/sticky_mark_sweep.cc | 13 +++++++++++++ runtime/gc/collector/sticky_mark_sweep.h | 6 ++++++ runtime/runtime.h | 4 +--- test/141-class-unload/expected.txt | 1 + test/141-class-unload/src/Main.java | 26 ++++++++++++++++++++++++++ 8 files changed, 50 insertions(+), 7 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index a34e029089..9e6519ba97 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -1848,7 +1848,7 @@ void ClassLinker::VisitClassRoots(RootVisitor* visitor, VisitRootFlags flags) { boot_class_table_.VisitRoots(buffered_visitor); // If tracing is enabled, then mark all the class loaders to prevent unloading. - if (tracing_enabled) { + if ((flags & kVisitRootFlagClassLoader) != 0 || tracing_enabled) { for (const ClassLoaderData& data : class_loaders_) { GcRoot root(GcRoot(self->DecodeJObject(data.weak_root))); root.VisitRoot(visitor, RootInfo(kRootVMInternal)); diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc index 24cbf10d64..4081ec7a7e 100644 --- a/runtime/gc/collector/mark_sweep.cc +++ b/runtime/gc/collector/mark_sweep.cc @@ -627,8 +627,7 @@ void MarkSweep::MarkNonThreadRoots() { void MarkSweep::MarkConcurrentRoots(VisitRootFlags flags) { TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings()); // Visit all runtime roots and clear dirty flags. - Runtime::Current()->VisitConcurrentRoots( - this, static_cast(flags | kVisitRootFlagNonMoving)); + Runtime::Current()->VisitConcurrentRoots(this, flags); } class MarkSweep::DelayReferenceReferentVisitor { diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h index 92d7cf6056..f4c7ba6c9b 100644 --- a/runtime/gc/collector/mark_sweep.h +++ b/runtime/gc/collector/mark_sweep.h @@ -98,7 +98,7 @@ class MarkSweep : public GarbageCollector { REQUIRES(!mark_stack_lock_) SHARED_REQUIRES(Locks::mutator_lock_); - void MarkConcurrentRoots(VisitRootFlags flags) + virtual void MarkConcurrentRoots(VisitRootFlags flags) REQUIRES(Locks::heap_bitmap_lock_) REQUIRES(!mark_stack_lock_) SHARED_REQUIRES(Locks::mutator_lock_); diff --git a/runtime/gc/collector/sticky_mark_sweep.cc b/runtime/gc/collector/sticky_mark_sweep.cc index bb7e854ea1..a2dbe3f7a0 100644 --- a/runtime/gc/collector/sticky_mark_sweep.cc +++ b/runtime/gc/collector/sticky_mark_sweep.cc @@ -56,6 +56,19 @@ void StickyMarkSweep::MarkReachableObjects() { RecursiveMarkDirtyObjects(false, accounting::CardTable::kCardDirty - 1); } +void StickyMarkSweep::MarkConcurrentRoots(VisitRootFlags flags) { + TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings()); + // Visit all runtime roots and clear dirty flags including class loader. This is done to prevent + // incorrect class unloading since the GC does not card mark when storing store the class during + // object allocation. Doing this for each allocation would be slow. + // Since the card is not dirty, it means the object may not get scanned. This can cause class + // unloading to occur even though the class and class loader are reachable through the object's + // class. + Runtime::Current()->VisitConcurrentRoots( + this, + static_cast(flags | kVisitRootFlagClassLoader)); +} + void StickyMarkSweep::Sweep(bool swap_bitmaps ATTRIBUTE_UNUSED) { SweepArray(GetHeap()->GetLiveStack(), false); } diff --git a/runtime/gc/collector/sticky_mark_sweep.h b/runtime/gc/collector/sticky_mark_sweep.h index abaf97845d..93b0b5fa79 100644 --- a/runtime/gc/collector/sticky_mark_sweep.h +++ b/runtime/gc/collector/sticky_mark_sweep.h @@ -33,6 +33,12 @@ class StickyMarkSweep FINAL : public PartialMarkSweep { StickyMarkSweep(Heap* heap, bool is_concurrent, const std::string& name_prefix = ""); ~StickyMarkSweep() {} + void MarkConcurrentRoots(VisitRootFlags flags) + OVERRIDE + REQUIRES(!mark_stack_lock_) + REQUIRES(Locks::heap_bitmap_lock_) + SHARED_REQUIRES(Locks::mutator_lock_); + protected: // Bind the live bits to the mark bits of bitmaps for all spaces, all spaces other than the // alloc space will be marked as immune. diff --git a/runtime/runtime.h b/runtime/runtime.h index 3b72aa7b38..f0cbed67c9 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -105,9 +105,7 @@ enum VisitRootFlags : uint8_t { kVisitRootFlagStartLoggingNewRoots = 0x4, kVisitRootFlagStopLoggingNewRoots = 0x8, kVisitRootFlagClearRootLog = 0x10, - // Non moving means we can have optimizations where we don't visit some roots if they are - // definitely reachable from another location. E.g. ArtMethod and ArtField roots. - kVisitRootFlagNonMoving = 0x20, + kVisitRootFlagClassLoader = 0x20, }; class Runtime { diff --git a/test/141-class-unload/expected.txt b/test/141-class-unload/expected.txt index 11de660c43..ae5f51186f 100644 --- a/test/141-class-unload/expected.txt +++ b/test/141-class-unload/expected.txt @@ -22,3 +22,4 @@ JNI_OnLoad called class null false test JNI_OnUnload called Number of loaded unload-ex maps 0 +Too small false diff --git a/test/141-class-unload/src/Main.java b/test/141-class-unload/src/Main.java index 17a6049dbf..3a05302fd7 100644 --- a/test/141-class-unload/src/Main.java +++ b/test/141-class-unload/src/Main.java @@ -49,6 +49,8 @@ public class Main { stressTest(constructor); // Test that the oat files are unloaded. testOatFilesUnloaded(getPid()); + // Test that objects keep class loader live for sticky GC. + testStickyUnload(constructor); } catch (Exception e) { e.printStackTrace(); } @@ -152,6 +154,30 @@ public class Main { return new WeakReference(intHolder); } + private static Object allocObjectInOtherClassLoader(Constructor constructor) + throws Exception { + ClassLoader loader = (ClassLoader) constructor.newInstance( + DEX_FILE, LIBRARY_SEARCH_PATH, ClassLoader.getSystemClassLoader()); + return loader.loadClass("IntHolder").newInstance(); + } + + // Regression test for public issue 227182. + private static void testStickyUnload(Constructor constructor) throws Exception { + String s = ""; + for (int i = 0; i < 10; ++i) { + s = ""; + // The object is the only thing preventing the class loader from being unloaded. + Object o = allocObjectInOtherClassLoader(constructor); + for (int j = 0; j < 1000; ++j) { + s += j + " "; + } + // Make sure the object still has a valid class (hasn't been incorrectly unloaded). + s += o.getClass().getName(); + o = null; + } + System.out.println("Too small " + (s.length() < 1000)); + } + private static WeakReference setUpUnloadLoader(Constructor constructor, boolean waitForCompilation) throws Exception { -- cgit v1.2.3 From 61049e87018d5c7420f14a552a726fd66249ace3 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Mon, 9 Jan 2017 18:48:11 -0800 Subject: Delete extra arm64/mips64 MterpReturn suspend check Doing a suspend check after moving the result into the shadow frame result_register_ is not safe since result_register_ is not a GC root. The suspend check is unnecessary since the opcodes that branch to MterpReturn already do a suspend check. This could maybe explain one crash for CC that was seen after calling a getter that had no compiled code. The extra suspend check appears to only be present on arm64 amd mips64. Test: test-art-target ART_TEST_INTERPRETER=true, N5X booting (cherry picked from commit aceff18580b94a586a469110565f2ba166f3635a) Bug: 33211261 Change-Id: I70b8863f40a25a26f278ac8ef0d57e083b663e0f --- runtime/interpreter/mterp/arm64/footer.S | 6 ------ runtime/interpreter/mterp/mips64/footer.S | 6 ------ runtime/interpreter/mterp/out/mterp_arm64.S | 6 ------ runtime/interpreter/mterp/out/mterp_mips64.S | 6 ------ 4 files changed, 24 deletions(-) diff --git a/runtime/interpreter/mterp/arm64/footer.S b/runtime/interpreter/mterp/arm64/footer.S index 2d3a11eafa..dbcbc71469 100644 --- a/runtime/interpreter/mterp/arm64/footer.S +++ b/runtime/interpreter/mterp/arm64/footer.S @@ -267,13 +267,7 @@ MterpExceptionReturn: b MterpDone MterpReturn: ldr x2, [xFP, #OFF_FP_RESULT_REGISTER] - ldr lr, [xSELF, #THREAD_FLAGS_OFFSET] str x0, [x2] - mov x0, xSELF - ands lr, lr, #(THREAD_SUSPEND_REQUEST | THREAD_CHECKPOINT_REQUEST) - b.eq check2 - bl MterpSuspendCheck // (self) -check2: mov x0, #1 // signal return to caller. MterpDone: /* diff --git a/runtime/interpreter/mterp/mips64/footer.S b/runtime/interpreter/mterp/mips64/footer.S index 14d5fe01f5..0545194dba 100644 --- a/runtime/interpreter/mterp/mips64/footer.S +++ b/runtime/interpreter/mterp/mips64/footer.S @@ -134,13 +134,7 @@ MterpExceptionReturn: */ MterpReturn: ld a2, OFF_FP_RESULT_REGISTER(rFP) - lw ra, THREAD_FLAGS_OFFSET(rSELF) sd a0, 0(a2) - move a0, rSELF - and ra, ra, (THREAD_SUSPEND_REQUEST | THREAD_CHECKPOINT_REQUEST) - beqzc ra, check2 - jal MterpSuspendCheck # (self) -check2: li v0, 1 # signal return to caller. MterpDone: ld s5, STACK_OFFSET_S5(sp) diff --git a/runtime/interpreter/mterp/out/mterp_arm64.S b/runtime/interpreter/mterp/out/mterp_arm64.S index 55797e676f..33c1abd868 100644 --- a/runtime/interpreter/mterp/out/mterp_arm64.S +++ b/runtime/interpreter/mterp/out/mterp_arm64.S @@ -11554,13 +11554,7 @@ MterpExceptionReturn: b MterpDone MterpReturn: ldr x2, [xFP, #OFF_FP_RESULT_REGISTER] - ldr lr, [xSELF, #THREAD_FLAGS_OFFSET] str x0, [x2] - mov x0, xSELF - ands lr, lr, #(THREAD_SUSPEND_REQUEST | THREAD_CHECKPOINT_REQUEST) - b.eq check2 - bl MterpSuspendCheck // (self) -check2: mov x0, #1 // signal return to caller. MterpDone: /* diff --git a/runtime/interpreter/mterp/out/mterp_mips64.S b/runtime/interpreter/mterp/out/mterp_mips64.S index a17252b2f8..b96b4bdd1c 100644 --- a/runtime/interpreter/mterp/out/mterp_mips64.S +++ b/runtime/interpreter/mterp/out/mterp_mips64.S @@ -12386,13 +12386,7 @@ MterpExceptionReturn: */ MterpReturn: ld a2, OFF_FP_RESULT_REGISTER(rFP) - lw ra, THREAD_FLAGS_OFFSET(rSELF) sd a0, 0(a2) - move a0, rSELF - and ra, ra, (THREAD_SUSPEND_REQUEST | THREAD_CHECKPOINT_REQUEST) - beqzc ra, check2 - jal MterpSuspendCheck # (self) -check2: li v0, 1 # signal return to caller. MterpDone: ld s5, STACK_OFFSET_S5(sp) -- cgit v1.2.3 From 3f3d4d63199e57cb1dccad62d2d3ef69fc170c06 Mon Sep 17 00:00:00 2001 From: Artem Udovichenko Date: Thu, 17 Nov 2016 10:51:58 +0300 Subject: Cache flush/invalidate needs RWX permission When generating JIT code, perform cache maintenance operations before removing page write permissions. Errata on some cores require data flush operations to be followed by data invalidate operations requiring write permission. Test: ART_TEST_JIT=true test-art-target on an arm64 device. bug: 27265969 (cherry picked from commit b18a669259aa6ba08d9ca01b5b32c1aa0417138b) Change-Id: I53080c664f9e7cfebe25f87cf6a45cd6eb33b281 --- runtime/jit/jit_code_cache.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc index 8c1d7768ca..d00ca5af61 100644 --- a/runtime/jit/jit_code_cache.cc +++ b/runtime/jit/jit_code_cache.cc @@ -342,10 +342,16 @@ uint8_t* JitCodeCache::CommitCodeInternal(Thread* self, core_spill_mask, fp_spill_mask, code_size); + // Flush caches before we remove write permission because on some ARMv8 hardware, + // flushing caches require write permissions. + // + // For reference, here are kernel patches discussing about this issue: + // https://android.googlesource.com/kernel/msm/%2B/0e7f7bcc3fc87489cda5aa6aff8ce40eed912279 + // https://patchwork.kernel.org/patch/9047921/ + FlushInstructionCache(reinterpret_cast(code_ptr), + reinterpret_cast(code_ptr + code_size)); } - FlushInstructionCache(reinterpret_cast(code_ptr), - reinterpret_cast(code_ptr + code_size)); number_of_compilations_++; } // We need to update the entry point in the runnable state for the instrumentation. -- cgit v1.2.3 From f5f462b8e448c13d30094d0d259d8b06bc525376 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Thu, 15 Dec 2016 17:57:38 +0000 Subject: Add Thread entry to signal if the thread can call into java Compiler threads (AOT or JIT) should not call into Java as they have no peers (which may lead to crashes, e.g. b/33067273) (cherry picked from commit ccd56958eb46fbb00c1eb45c7a7b23d5bbfd7698) Bug: 32602185 Bug: 33067273 Test: m test-art-host-run-test; m test-art-host-gtest Change-Id: I97dda7a5444643db3c5d5318339a65a602f709e8 --- runtime/class_linker.cc | 8 ++++++++ runtime/runtime.cc | 2 ++ runtime/thread.cc | 6 +++++- runtime/thread.h | 13 +++++++++++++ runtime/thread_pool.cc | 2 ++ runtime/thread_pool.h | 1 + 6 files changed, 31 insertions(+), 1 deletion(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 9e6519ba97..a88291f28f 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -2409,6 +2409,14 @@ mirror::Class* ClassLinker::FindClass(Thread* self, // We'll let the Java-side rediscover all this and throw the exception with the right stack // trace. + if (!self->CanCallIntoJava()) { + // Oops, we can't call into java so we can't run actual class-loader code. + // This is true for e.g. for the compiler (jit or aot). + mirror::Throwable* pre_allocated = + Runtime::Current()->GetPreAllocatedNoClassDefFoundError(); + self->SetException(pre_allocated); + return nullptr; + } } if (Runtime::Current()->IsAotCompiler()) { diff --git a/runtime/runtime.cc b/runtime/runtime.cc index f4c28b9a53..4300cd23ff 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -1146,6 +1146,8 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { CHECK_EQ(self->GetThreadId(), ThreadList::kMainThreadId); CHECK(self != nullptr); + self->SetCanCallIntoJava(!IsAotCompiler()); + // Set us to runnable so tools using a runtime can allocate and GC by default self->TransitionFromSuspendedToRunnable(); diff --git a/runtime/thread.cc b/runtime/thread.cc index 16f30cded9..f3b39c9d14 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1602,7 +1602,11 @@ void Thread::Shutdown() { } } -Thread::Thread(bool daemon) : tls32_(daemon), wait_monitor_(nullptr), interrupted_(false) { +Thread::Thread(bool daemon) + : tls32_(daemon), + wait_monitor_(nullptr), + interrupted_(false), + can_call_into_java_(true) { wait_mutex_ = new Mutex("a thread wait mutex"); wait_cond_ = new ConditionVariable("a thread wait condition variable", *wait_mutex_); tlsPtr_.instrumentation_stack = new std::deque; diff --git a/runtime/thread.h b/runtime/thread.h index 582a0cdbd6..4c284247ac 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -864,6 +864,15 @@ class Thread { --tls32_.disable_thread_flip_count; } + // Returns true if the thread is allowed to call into java. + bool CanCallIntoJava() const { + return can_call_into_java_; + } + + void SetCanCallIntoJava(bool can_call_into_java) { + can_call_into_java_ = can_call_into_java; + } + // Activates single step control for debugging. The thread takes the // ownership of the given SingleStepControl*. It is deleted by a call // to DeactivateSingleStepControl or upon thread destruction. @@ -1518,6 +1527,10 @@ class Thread { // Debug disable read barrier count, only is checked for debug builds and only in the runtime. uint8_t debug_disallow_read_barrier_ = 0; + // True if the thread is allowed to call back into java (for e.g. during class resolution). + // By default this is true. + bool can_call_into_java_; + friend class Dbg; // For SetStateUnsafe. friend class gc::collector::SemiSpace; // For getting stack traces. friend class Runtime; // For CreatePeer. diff --git a/runtime/thread_pool.cc b/runtime/thread_pool.cc index 2fba805fa2..06476a5273 100644 --- a/runtime/thread_pool.cc +++ b/runtime/thread_pool.cc @@ -85,6 +85,8 @@ void* ThreadPoolWorker::Callback(void* arg) { ThreadPoolWorker* worker = reinterpret_cast(arg); Runtime* runtime = Runtime::Current(); CHECK(runtime->AttachCurrentThread(worker->name_.c_str(), true, nullptr, false)); + // Thread pool workers cannot call into java. + Thread::Current()->SetCanCallIntoJava(false); // Do work until its time to shut down. worker->Run(); runtime->DetachCurrentThread(); diff --git a/runtime/thread_pool.h b/runtime/thread_pool.h index b6c6f02db8..adcb817db4 100644 --- a/runtime/thread_pool.h +++ b/runtime/thread_pool.h @@ -77,6 +77,7 @@ class ThreadPoolWorker { DISALLOW_COPY_AND_ASSIGN(ThreadPoolWorker); }; +// Note that thread pool workers will set Thread#setCanCallIntoJava to false. class ThreadPool { public: // Returns the number of threads in the thread pool. -- cgit v1.2.3 From 2db6c407e948941cead24e4fc0907716ffcd45a1 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 24 Jan 2017 13:12:19 -0800 Subject: ART: Add ThreadPool mode that creates peers Add a mode where worker threads in a thread pool will get a Java peer. Add a test. (cherry picked from commit b15de0c0580633701c19c32bb60bcd64f30da867) Bug: 29547798 Bug: 31684920 Test: m test-art-host-gtest-thread_pool_test Change-Id: I3654cc2be1294a79881b6ac9f84445a1e7f24b70 --- runtime/thread_pool.cc | 11 +++++++--- runtime/thread_pool.h | 9 +++++++- runtime/thread_pool_test.cc | 52 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/runtime/thread_pool.cc b/runtime/thread_pool.cc index 06476a5273..c03fb2eea5 100644 --- a/runtime/thread_pool.cc +++ b/runtime/thread_pool.cc @@ -84,7 +84,10 @@ void ThreadPoolWorker::Run() { void* ThreadPoolWorker::Callback(void* arg) { ThreadPoolWorker* worker = reinterpret_cast(arg); Runtime* runtime = Runtime::Current(); - CHECK(runtime->AttachCurrentThread(worker->name_.c_str(), true, nullptr, false)); + CHECK(runtime->AttachCurrentThread(worker->name_.c_str(), + true, + nullptr, + worker->thread_pool_->create_peers_)); // Thread pool workers cannot call into java. Thread::Current()->SetCanCallIntoJava(false); // Do work until its time to shut down. @@ -107,7 +110,7 @@ void ThreadPool::RemoveAllTasks(Thread* self) { tasks_.clear(); } -ThreadPool::ThreadPool(const char* name, size_t num_threads) +ThreadPool::ThreadPool(const char* name, size_t num_threads, bool create_peers) : name_(name), task_queue_lock_("task queue lock"), task_queue_condition_("task queue condition", task_queue_lock_), @@ -119,7 +122,8 @@ ThreadPool::ThreadPool(const char* name, size_t num_threads) total_wait_time_(0), // Add one since the caller of constructor waits on the barrier too. creation_barier_(num_threads + 1), - max_active_workers_(num_threads) { + max_active_workers_(num_threads), + create_peers_(create_peers) { Thread* self = Thread::Current(); while (GetThreadCount() < num_threads) { const std::string worker_name = StringPrintf("%s worker thread %zu", name_.c_str(), @@ -212,6 +216,7 @@ Task* ThreadPool::TryGetTaskLocked() { void ThreadPool::Wait(Thread* self, bool do_work, bool may_hold_locks) { if (do_work) { + CHECK(!create_peers_); Task* task = nullptr; while ((task = TryGetTask(self)) != nullptr) { task->Run(self); diff --git a/runtime/thread_pool.h b/runtime/thread_pool.h index adcb817db4..739ac66ec9 100644 --- a/runtime/thread_pool.h +++ b/runtime/thread_pool.h @@ -98,10 +98,16 @@ class ThreadPool { // Remove all tasks in the queue. void RemoveAllTasks(Thread* self) REQUIRES(!task_queue_lock_); - ThreadPool(const char* name, size_t num_threads); + // Create a named thread pool with the given number of threads. + // + // If create_peers is true, all worker threads will have a Java peer object. Note that if the + // pool is asked to do work on the current thread (see Wait), a peer may not be available. Wait + // will conservatively abort if create_peers and do_work are true. + ThreadPool(const char* name, size_t num_threads, bool create_peers = false); virtual ~ThreadPool(); // Wait for all tasks currently on queue to get completed. + // When the pool was created with peers for workers, do_work must not be true (see ThreadPool()). void Wait(Thread* self, bool do_work, bool may_hold_locks) REQUIRES(!task_queue_lock_); size_t GetTaskCount(Thread* self) REQUIRES(!task_queue_lock_); @@ -147,6 +153,7 @@ class ThreadPool { uint64_t total_wait_time_; Barrier creation_barier_; size_t max_active_workers_ GUARDED_BY(task_queue_lock_); + const bool create_peers_; private: friend class ThreadPoolWorker; diff --git a/runtime/thread_pool_test.cc b/runtime/thread_pool_test.cc index d5f17d17b1..23d04d1959 100644 --- a/runtime/thread_pool_test.cc +++ b/runtime/thread_pool_test.cc @@ -20,6 +20,7 @@ #include "atomic.h" #include "common_runtime_test.h" +#include "scoped_thread_state_change.h" #include "thread-inl.h" namespace art { @@ -136,4 +137,55 @@ TEST_F(ThreadPoolTest, RecursiveTest) { EXPECT_EQ((1 << depth) - 1, count.LoadSequentiallyConsistent()); } +class PeerTask : public Task { + public: + PeerTask() {} + + void Run(Thread* self) { + ScopedObjectAccess soa(self); + CHECK(self->GetPeer() != nullptr); + } + + void Finalize() { + delete this; + } +}; + +class NoPeerTask : public Task { + public: + NoPeerTask() {} + + void Run(Thread* self) { + ScopedObjectAccess soa(self); + CHECK(self->GetPeer() == nullptr); + } + + void Finalize() { + delete this; + } +}; + +// Tests for create_peer functionality. +TEST_F(ThreadPoolTest, PeerTest) { + Thread* self = Thread::Current(); + { + ThreadPool thread_pool("Thread pool test thread pool", 1); + thread_pool.AddTask(self, new NoPeerTask()); + thread_pool.StartWorkers(self); + thread_pool.Wait(self, false, false); + } + + { + // To create peers, the runtime needs to be started. + self->TransitionFromSuspendedToRunnable(); + bool started = runtime_->Start(); + ASSERT_TRUE(started); + + ThreadPool thread_pool("Thread pool test thread pool", 1, true); + thread_pool.AddTask(self, new PeerTask()); + thread_pool.StartWorkers(self); + thread_pool.Wait(self, false, false); + } +} + } // namespace art -- cgit v1.2.3 From f9e3754e8a2f6244e037362c03db32d4244ecb2d Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Mon, 30 Jan 2017 16:40:49 +0000 Subject: Revert "Revert "ART: Give JIT thread pool workers a peer"" This reverts commit 9dfb707ba2f8c2ff67d42c26e3214f5d7142b6d3. Accept a live Java thread for the JIT, and adjust the tests accordingly. (cherry picked from commit 4471e4f7c5874bdaf93762b6047d4a4bebc465df) Bug: 31684920 Test: m ART_TEST_JIT=true ART_TEST_INTERPRETER=true test-art-host Change-Id: I92cbae1eaae05711b9069335cf1a5f7eb58b9fd8 --- runtime/jit/jit.cc | 13 +++++++++++-- runtime/runtime.cc | 39 +++++++++++++++++++++------------------ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc index 74d99176c4..8cd867d0ba 100644 --- a/runtime/jit/jit.cc +++ b/runtime/jit/jit.cc @@ -145,7 +145,12 @@ Jit::Jit() : dump_info_on_shutdown_(false), memory_use_("Memory used for compilation", 16), lock_("JIT memory use lock"), use_jit_compilation_(true), - save_profiling_info_(false) {} + save_profiling_info_(false), + hot_method_threshold_(0), + warm_method_threshold_(0), + osr_method_threshold_(0), + priority_thread_weight_(0), + invoke_transition_weight_(0) {} Jit* Jit::Create(JitOptions* options, std::string* error_msg) { DCHECK(options->UseJitCompilation() || options->GetSaveProfilingInfo()); @@ -280,7 +285,11 @@ bool Jit::CompileMethod(ArtMethod* method, Thread* self, bool osr) { void Jit::CreateThreadPool() { // There is a DCHECK in the 'AddSamples' method to ensure the tread pool // is not null when we instrument. - thread_pool_.reset(new ThreadPool("Jit thread pool", 1)); + + // We need peers as we may report the JIT thread, e.g., in the debugger. + constexpr bool kJitPoolNeedsPeers = true; + thread_pool_.reset(new ThreadPool("Jit thread pool", 1, kJitPoolNeedsPeers)); + thread_pool_->SetPthreadPriority(kJitPoolThreadPthreadPriority); thread_pool_->StartWorkers(Thread::Current()); } diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 4300cd23ff..6d0d6ed7fc 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -583,24 +583,6 @@ bool Runtime::Start() { started_ = true; - // Create the JIT either if we have to use JIT compilation or save profiling info. - // TODO(calin): We use the JIT class as a proxy for JIT compilation and for - // recoding profiles. Maybe we should consider changing the name to be more clear it's - // not only about compiling. b/28295073. - if (jit_options_->UseJitCompilation() || jit_options_->GetSaveProfilingInfo()) { - std::string error_msg; - if (!IsZygote()) { - // If we are the zygote then we need to wait until after forking to create the code cache - // due to SELinux restrictions on r/w/x memory regions. - CreateJit(); - } else if (jit_options_->UseJitCompilation()) { - if (!jit::Jit::LoadCompilerLibrary(&error_msg)) { - // Try to load compiler pre zygote to reduce PSS. b/27744947 - LOG(WARNING) << "Failed to load JIT compiler with error " << error_msg; - } - } - } - if (!IsImageDex2OatEnabled() || !GetHeap()->HasBootImageSpace()) { ScopedObjectAccess soa(self); StackHandleScope<2> hs(soa.Self()); @@ -625,6 +607,27 @@ bool Runtime::Start() { Thread::FinishStartup(); + // Create the JIT either if we have to use JIT compilation or save profiling info. This is + // done after FinishStartup as the JIT pool needs Java thread peers, which require the main + // ThreadGroup to exist. + // + // TODO(calin): We use the JIT class as a proxy for JIT compilation and for + // recoding profiles. Maybe we should consider changing the name to be more clear it's + // not only about compiling. b/28295073. + if (jit_options_->UseJitCompilation() || jit_options_->GetSaveProfilingInfo()) { + std::string error_msg; + if (!IsZygote()) { + // If we are the zygote then we need to wait until after forking to create the code cache + // due to SELinux restrictions on r/w/x memory regions. + CreateJit(); + } else if (jit_options_->UseJitCompilation()) { + if (!jit::Jit::LoadCompilerLibrary(&error_msg)) { + // Try to load compiler pre zygote to reduce PSS. b/27744947 + LOG(WARNING) << "Failed to load JIT compiler with error " << error_msg; + } + } + } + system_class_loader_ = CreateSystemClassLoader(this); if (is_zygote_) { -- cgit v1.2.3