diff options
| -rw-r--r-- | compiler/optimizing/intrinsics_arm64.cc | 3 | ||||
| -rw-r--r-- | compiler/optimizing/intrinsics_arm_vixl.cc | 4 | ||||
| -rw-r--r-- | compiler/optimizing/intrinsics_x86.cc | 5 | ||||
| -rw-r--r-- | compiler/optimizing/intrinsics_x86_64.cc | 5 | ||||
| -rw-r--r-- | runtime/gc/allocation_record.cc | 1 | ||||
| -rw-r--r-- | runtime/gc/collector/concurrent_copying.cc | 44 | ||||
| -rw-r--r-- | runtime/gc/collector/mark_sweep.cc | 10 | ||||
| -rw-r--r-- | runtime/gc/collector/semi_space.cc | 5 | ||||
| -rw-r--r-- | runtime/gc/reference_processor.cc | 263 | ||||
| -rw-r--r-- | runtime/gc/reference_processor.h | 43 | ||||
| -rw-r--r-- | runtime/gc/reference_queue.cc | 49 | ||||
| -rw-r--r-- | runtime/gc/reference_queue.h | 22 | ||||
| -rw-r--r-- | runtime/intern_table.cc | 1 | ||||
| -rw-r--r-- | runtime/intern_table.h | 2 | ||||
| -rw-r--r-- | runtime/interpreter/interpreter_common.h | 1 | ||||
| -rw-r--r-- | runtime/jit/jit_code_cache.cc | 1 | ||||
| -rw-r--r-- | runtime/jni/java_vm_ext.cc | 4 | ||||
| -rw-r--r-- | runtime/thread-inl.h | 19 | ||||
| -rw-r--r-- | runtime/thread.h | 44 |
19 files changed, 196 insertions, 330 deletions
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index 0236f0d5a9..77b55e455e 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -3475,8 +3475,7 @@ void IntrinsicCodeGeneratorARM64::VisitReferenceGetReferent(HInvoke* invoke) { Register temp = temps.AcquireW(); __ Ldr(temp, MemOperand(tr, Thread::WeakRefAccessEnabledOffset<kArm64PointerSize>().Uint32Value())); - static_assert(enum_cast<int32_t>(WeakRefAccessState::kVisiblyEnabled) == 0); - __ Cbnz(temp, slow_path->GetEntryLabel()); + __ Cbz(temp, slow_path->GetEntryLabel()); } { diff --git a/compiler/optimizing/intrinsics_arm_vixl.cc b/compiler/optimizing/intrinsics_arm_vixl.cc index 303ac171a7..a4a3457c37 100644 --- a/compiler/optimizing/intrinsics_arm_vixl.cc +++ b/compiler/optimizing/intrinsics_arm_vixl.cc @@ -2517,8 +2517,8 @@ void IntrinsicCodeGeneratorARMVIXL::VisitReferenceGetReferent(HInvoke* invoke) { vixl32::Register temp = temps.Acquire(); __ Ldr(temp, MemOperand(tr, Thread::WeakRefAccessEnabledOffset<kArmPointerSize>().Uint32Value())); - __ Cmp(temp, enum_cast<int32_t>(WeakRefAccessState::kVisiblyEnabled)); - __ B(ne, slow_path->GetEntryLabel()); + __ Cmp(temp, 0); + __ B(eq, slow_path->GetEntryLabel()); } { diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc index 3a3886432a..7c2537495a 100644 --- a/compiler/optimizing/intrinsics_x86.cc +++ b/compiler/optimizing/intrinsics_x86.cc @@ -3346,9 +3346,8 @@ void IntrinsicCodeGeneratorX86::VisitReferenceGetReferent(HInvoke* invoke) { if (kEmitCompilerReadBarrier) { // Check self->GetWeakRefAccessEnabled(). ThreadOffset32 offset = Thread::WeakRefAccessEnabledOffset<kX86PointerSize>(); - __ fs()->cmpl(Address::Absolute(offset), - Immediate(enum_cast<int32_t>(WeakRefAccessState::kVisiblyEnabled))); - __ j(kNotEqual, slow_path->GetEntryLabel()); + __ fs()->cmpl(Address::Absolute(offset), Immediate(0)); + __ j(kEqual, slow_path->GetEntryLabel()); } // Load the java.lang.ref.Reference class, use the output register as a temporary. diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc index e3be98732b..d5a7cb10e1 100644 --- a/compiler/optimizing/intrinsics_x86_64.cc +++ b/compiler/optimizing/intrinsics_x86_64.cc @@ -3098,9 +3098,8 @@ void IntrinsicCodeGeneratorX86_64::VisitReferenceGetReferent(HInvoke* invoke) { if (kEmitCompilerReadBarrier) { // Check self->GetWeakRefAccessEnabled(). ThreadOffset64 offset = Thread::WeakRefAccessEnabledOffset<kX86_64PointerSize>(); - __ gs()->cmpl(Address::Absolute(offset, /* no_rip= */ true), - Immediate(enum_cast<int32_t>(WeakRefAccessState::kVisiblyEnabled))); - __ j(kNotEqual, slow_path->GetEntryLabel()); + __ gs()->cmpl(Address::Absolute(offset, /* no_rip= */ true), Immediate(0)); + __ j(kEqual, slow_path->GetEntryLabel()); } // Load the java.lang.ref.Reference class, use the output register as a temporary. diff --git a/runtime/gc/allocation_record.cc b/runtime/gc/allocation_record.cc index ffd4d2b582..60fb71d952 100644 --- a/runtime/gc/allocation_record.cc +++ b/runtime/gc/allocation_record.cc @@ -23,7 +23,6 @@ #include "obj_ptr-inl.h" #include "object_callbacks.h" #include "stack.h" -#include "thread-inl.h" // For GetWeakRefAccessEnabled(). #include <android-base/properties.h> diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc index 34a4089702..3a60191310 100644 --- a/runtime/gc/collector/concurrent_copying.cc +++ b/runtime/gc/collector/concurrent_copying.cc @@ -1621,28 +1621,18 @@ void ConcurrentCopying::CopyingPhase() { { TimingLogger::ScopedTiming split7("Process mark stacks and References", GetTimings()); - - // Process the mark stack once in the thread local stack mode. This marks most of the live - // objects, aside from weak ref accesses with read barriers (Reference::GetReferent() and - // system weaks) that may happen concurrently while we are processing the mark stack and newly - // mark/gray objects and push refs on the mark stack. - ProcessMarkStack(); - - ReferenceProcessor* rp = GetHeap()->GetReferenceProcessor(); - bool clear_soft_references = GetCurrentIteration()->GetClearSoftReferences(); - rp->Setup(self, this, /*concurrent=*/ true, clear_soft_references); - if (!clear_soft_references) { - // Forward as many SoftReferences as possible before inhibiting reference access. - rp->ForwardSoftReferences(GetTimings()); - } - // We transition through three mark stack modes (thread-local, shared, GC-exclusive). The - // primary reasons are that we need to use a checkpoint to process thread-local mark + // primary reasons are the fact that we need to use a checkpoint to process thread-local mark // stacks, but after we disable weak refs accesses, we can't use a checkpoint due to a deadlock // issue because running threads potentially blocking at WaitHoldingLocks, and that once we // reach the point where we process weak references, we can avoid using a lock when accessing // the GC mark stack, which makes mark stack processing more efficient. + // Process the mark stack once in the thread local stack mode. This marks most of the live + // objects, aside from weak ref accesses with read barriers (Reference::GetReferent() and system + // weaks) that may happen concurrently while we processing the mark stack and newly mark/gray + // objects and push refs on the mark stack. + ProcessMarkStack(); // Switch to the shared mark stack mode. That is, revoke and process thread-local mark stacks // for the last time before transitioning to the shared mark stack mode, which would process new // refs that may have been concurrently pushed onto the mark stack during the ProcessMarkStack() @@ -1659,7 +1649,6 @@ void ConcurrentCopying::CopyingPhase() { // forever as the checkpoint never finishes (See runtime/mutator_gc_coord.md). SwitchToSharedMarkStackMode(); CHECK(!self->GetWeakRefAccessEnabled()); - // Now that weak refs accesses are disabled, once we exhaust the shared mark stack again here // (which may be non-empty if there were refs found on thread-local mark stacks during the above // SwitchToSharedMarkStackMode() call), we won't have new refs to process, that is, mutators @@ -1667,7 +1656,6 @@ void ConcurrentCopying::CopyingPhase() { // before we process weak refs below. ProcessMarkStack(); CheckEmptyMarkStack(); - // Switch to the GC exclusive mark stack mode so that we can process the mark stack without a // lock from this point on. SwitchToGcExclusiveMarkStackMode(); @@ -1675,23 +1663,24 @@ void ConcurrentCopying::CopyingPhase() { if (kVerboseMode) { LOG(INFO) << "ProcessReferences"; } - // Process weak references. This also marks through finalizers. Although - // reference processing is "disabled", some accesses will proceed once we've ensured that - // objects directly reachable by the mutator are marked, i.e. before we mark through - // finalizers. + // Process weak references. This may produce new refs to process and have them processed via + // ProcessMarkStack (in the GC exclusive mark stack mode). ProcessReferences(self); CheckEmptyMarkStack(); - // JNI WeakGlobalRefs and most other system weaks cannot be processed until we're done marking - // through finalizers, since such references to finalizer-reachable objects must be preserved. if (kVerboseMode) { LOG(INFO) << "SweepSystemWeaks"; } SweepSystemWeaks(self); - CheckEmptyMarkStack(); - ReenableWeakRefAccess(self); if (kVerboseMode) { LOG(INFO) << "SweepSystemWeaks done"; } + // Process the mark stack here one last time because the above SweepSystemWeaks() call may have + // marked some objects (strings alive) as hash_set::Erase() can call the hash function for + // arbitrary elements in the weak intern table in InternTable::Table::SweepWeaks(). + ProcessMarkStack(); + CheckEmptyMarkStack(); + // Re-enable weak ref accesses. + ReenableWeakRefAccess(self); // Free data for class loaders that we unloaded. Runtime::Current()->GetClassLinker()->CleanupClassLoaders(); // Marking is done. Disable marking. @@ -3841,7 +3830,8 @@ void ConcurrentCopying::DelayReferenceReferent(ObjPtr<mirror::Class> klass, void ConcurrentCopying::ProcessReferences(Thread* self) { // We don't really need to lock the heap bitmap lock as we use CAS to mark in bitmaps. WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); - GetHeap()->GetReferenceProcessor()->ProcessReferences(self, GetTimings()); + GetHeap()->GetReferenceProcessor()->ProcessReferences( + /*concurrent=*/ true, GetTimings(), GetCurrentIteration()->GetClearSoftReferences(), this); } void ConcurrentCopying::RevokeAllThreadLocalBuffers() { diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc index bd5ce37b2c..e3cd1eefe9 100644 --- a/runtime/gc/collector/mark_sweep.cc +++ b/runtime/gc/collector/mark_sweep.cc @@ -177,7 +177,11 @@ void MarkSweep::RunPhases() { void MarkSweep::ProcessReferences(Thread* self) { WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); - GetHeap()->GetReferenceProcessor()->ProcessReferences(self, GetTimings()); + GetHeap()->GetReferenceProcessor()->ProcessReferences( + true, + GetTimings(), + GetCurrentIteration()->GetClearSoftReferences(), + this); } void MarkSweep::PausePhase() { @@ -209,9 +213,7 @@ void MarkSweep::PausePhase() { Runtime::Current()->DisallowNewSystemWeaks(); // Enable the reference processing slow path, needs to be done with mutators paused since there // is no lock in the GetReferent fast path. - ReferenceProcessor* rp = GetHeap()->GetReferenceProcessor(); - rp->Setup(self, this, /*concurrent=*/true, GetCurrentIteration()->GetClearSoftReferences()); - rp->EnableSlowPath(); + GetHeap()->GetReferenceProcessor()->EnableSlowPath(); } void MarkSweep::PreCleanCards() { diff --git a/runtime/gc/collector/semi_space.cc b/runtime/gc/collector/semi_space.cc index 53b060483f..0848f34d0b 100644 --- a/runtime/gc/collector/semi_space.cc +++ b/runtime/gc/collector/semi_space.cc @@ -145,9 +145,8 @@ void SemiSpace::InitializePhase() { void SemiSpace::ProcessReferences(Thread* self) { WriterMutexLock mu(self, *Locks::heap_bitmap_lock_); - ReferenceProcessor* rp = GetHeap()->GetReferenceProcessor(); - rp->Setup(self, this, /*concurrent=*/false, GetCurrentIteration()->GetClearSoftReferences()); - rp->ProcessReferences(self, GetTimings()); + GetHeap()->GetReferenceProcessor()->ProcessReferences( + false, GetTimings(), GetCurrentIteration()->GetClearSoftReferences(), this); } void SemiSpace::MarkingPhase() { diff --git a/runtime/gc/reference_processor.cc b/runtime/gc/reference_processor.cc index df8b3fed64..e34d140db4 100644 --- a/runtime/gc/reference_processor.cc +++ b/runtime/gc/reference_processor.cc @@ -32,7 +32,6 @@ #include "reflection.h" #include "scoped_thread_state_change-inl.h" #include "task_processor.h" -#include "thread-inl.h" #include "thread_pool.h" #include "well_known_classes.h" @@ -43,6 +42,7 @@ static constexpr bool kAsyncReferenceQueueAdd = false; ReferenceProcessor::ReferenceProcessor() : collector_(nullptr), + preserving_references_(false), condition_("reference processor condition", *Locks::reference_processor_lock_) , soft_reference_queue_(Locks::reference_queue_soft_references_lock_), weak_reference_queue_(Locks::reference_queue_weak_references_lock_), @@ -89,20 +89,17 @@ void ReferenceProcessor::BroadcastForSlowPath(Thread* self) { ObjPtr<mirror::Object> ReferenceProcessor::GetReferent(Thread* self, ObjPtr<mirror::Reference> reference) { - auto slow_path_required = [this, self]() { - return kUseReadBarrier ? !self->GetWeakRefAccessEnabled() : SlowPathEnabled(); - }; - if (!slow_path_required()) { - return reference->GetReferent(); - } - // If the referent is null then it is already cleared, we can just return null since there is no - // scenario where it becomes non-null during the reference processing phase. - // A read barrier may be unsafe here, and we use the result only when it's null or marked. - ObjPtr<mirror::Object> referent = reference->template GetReferent<kWithoutReadBarrier>(); - if (referent.IsNull()) { - return referent; + if (!kUseReadBarrier || self->GetWeakRefAccessEnabled()) { + // Under read barrier / concurrent copying collector, it's not safe to call GetReferent() when + // weak ref access is disabled as the call includes a read barrier which may push a ref onto the + // mark stack and interfere with termination of marking. + const ObjPtr<mirror::Object> referent = reference->GetReferent(); + // If the referent is null then it is already cleared, we can just return null since there is no + // scenario where it becomes non-null during the reference processing phase. + if (UNLIKELY(!SlowPathEnabled()) || referent == nullptr) { + return referent; + } } - bool started_trace = false; uint64_t start_millis; auto finish_trace = [](uint64_t start_millis) { @@ -115,47 +112,50 @@ ObjPtr<mirror::Object> ReferenceProcessor::GetReferent(Thread* self, }; MutexLock mu(self, *Locks::reference_processor_lock_); - // Keeping reference_processor_lock_ blocks the broadcast when we try to reenable the fast path. - while (slow_path_required()) { - DCHECK(collector_ != nullptr); - constexpr bool kOtherReadBarrier = kUseReadBarrier && !kUseBakerReadBarrier; - if (UNLIKELY(reference->IsFinalizerReferenceInstance() - || rp_state_ == RpState::kStarting /* too early to determine mark state */ - || (kOtherReadBarrier && reference->IsPhantomReferenceInstance()))) { - // Odd cases in which it doesn't hurt to just wait, or the wait is likely to be very brief. - - // Check and run the empty checkpoint before blocking so the empty checkpoint will work in the - // presence of threads blocking for weak ref access. - self->CheckEmptyCheckpointFromWeakRefAccess(Locks::reference_processor_lock_); - if (!started_trace) { - ATraceBegin("GetReferent blocked"); - started_trace = true; - start_millis = MilliTime(); + while ((!kUseReadBarrier && SlowPathEnabled()) || + (kUseReadBarrier && !self->GetWeakRefAccessEnabled())) { + ObjPtr<mirror::Object> referent = reference->GetReferent<kWithoutReadBarrier>(); + // If the referent became cleared, return it. Don't need barrier since thread roots can't get + // updated until after we leave the function due to holding the mutator lock. + if (referent == nullptr) { + if (started_trace) { + finish_trace(start_millis); } - condition_.WaitHoldingLocks(self); - continue; + return nullptr; } - DCHECK(!reference->IsPhantomReferenceInstance()); - - if (rp_state_ == RpState::kInitClearingDone) { - // Reachable references have their final referent values. - break; + // Try to see if the referent is already marked by using the is_marked_callback. We can return + // it to the mutator as long as the GC is not preserving references. + if (LIKELY(collector_ != nullptr)) { + // If it's null it means not marked, but it could become marked if the referent is reachable + // by finalizer referents. So we cannot return in this case and must block. Otherwise, we + // can return it to the mutator as long as the GC is not preserving references, in which + // case only black nodes can be safely returned. If the GC is preserving references, the + // mutator could take a white field from a grey or white node and move it somewhere else + // in the heap causing corruption since this field would get swept. + // Use the cached referent instead of calling GetReferent since other threads could call + // Reference.clear() after we did the null check resulting in a null pointer being + // incorrectly passed to IsMarked. b/33569625 + ObjPtr<mirror::Object> forwarded_ref = collector_->IsMarked(referent.Ptr()); + if (forwarded_ref != nullptr) { + // Non null means that it is marked. + if (!preserving_references_ || + (LIKELY(!reference->IsFinalizerReferenceInstance()) && reference->IsUnprocessed())) { + if (started_trace) { + finish_trace(start_millis); + } + return forwarded_ref; + } + } } - // Although reference processing is not done, we can always predict the correct return value - // based on the current mark state. No additional marking from finalizers has been done, since - // we hold reference_processor_lock_, which is required to advance to kInitClearingDone. - DCHECK(rp_state_ == RpState::kInitMarkingDone); - // Re-load and re-check referent, since the current one may have been read before we acquired - // reference_lock. In particular a Reference.clear() call may have intervened. (b/33569625) - referent = reference->GetReferent<kWithoutReadBarrier>(); - ObjPtr<mirror::Object> forwarded_ref = - referent.IsNull() ? nullptr : collector_->IsMarked(referent.Ptr()); - // Either the referent was marked, and forwarded_ref is the correct return value, or it - // was not, and forwarded_ref == null, which is again the correct return value. - if (started_trace) { - finish_trace(start_millis); + // Check and run the empty checkpoint before blocking so the empty checkpoint will work in the + // presence of threads blocking for weak ref access. + self->CheckEmptyCheckpointFromWeakRefAccess(Locks::reference_processor_lock_); + if (!started_trace) { + ATraceBegin("GetReferent blocked"); + started_trace = true; + start_millis = MilliTime(); } - return forwarded_ref; + condition_.WaitHoldingLocks(self); } if (started_trace) { finish_trace(start_millis); @@ -163,64 +163,36 @@ ObjPtr<mirror::Object> ReferenceProcessor::GetReferent(Thread* self, return reference->GetReferent(); } -// Forward SoftReferences. Can be done before we disable Reference access. Only -// invoked if we are not clearing SoftReferences. -uint32_t ReferenceProcessor::ForwardSoftReferences(TimingLogger* timings) { - TimingLogger::ScopedTiming split( - concurrent_ ? "ForwardSoftReferences" : "(Paused)ForwardSoftReferences", timings); - // We used to argue that we should be smarter about doing this conditionally, but it's unclear - // that's actually better than the more predictable strategy of basically only clearing - // SoftReferences just before we would otherwise run out of memory. - uint32_t non_null_refs = soft_reference_queue_.ForwardSoftReferences(collector_); - if (ATraceEnabled()) { - static constexpr size_t kBufSize = 80; - char buf[kBufSize]; - snprintf(buf, kBufSize, "Marking for %" PRIu32 " SoftReferences", non_null_refs); - ATraceBegin(buf); - collector_->ProcessMarkStack(); - ATraceEnd(); - } else { - collector_->ProcessMarkStack(); - } - return non_null_refs; +void ReferenceProcessor::StartPreservingReferences(Thread* self) { + MutexLock mu(self, *Locks::reference_processor_lock_); + preserving_references_ = true; } -void ReferenceProcessor::Setup(Thread* self, - collector::GarbageCollector* collector, - bool concurrent, - bool clear_soft_references) { - DCHECK(collector != nullptr); +void ReferenceProcessor::StopPreservingReferences(Thread* self) { MutexLock mu(self, *Locks::reference_processor_lock_); - collector_ = collector; - rp_state_ = RpState::kStarting; - concurrent_ = concurrent; - clear_soft_references_ = clear_soft_references; + preserving_references_ = false; + // We are done preserving references, some people who are blocked may see a marked referent. + condition_.Broadcast(self); } // Process reference class instances and schedule finalizations. -// We advance rp_state_ to signal partial completion for the benefit of GetReferent. -void ReferenceProcessor::ProcessReferences(Thread* self, TimingLogger* timings) { - TimingLogger::ScopedTiming t(concurrent_ ? __FUNCTION__ : "(Paused)ProcessReferences", timings); - if (!clear_soft_references_) { - // Forward any additional SoftReferences we discovered late, now that reference access has been - // inhibited. - while (!soft_reference_queue_.IsEmpty()) { - ForwardSoftReferences(timings); - } - } +void ReferenceProcessor::ProcessReferences(bool concurrent, + TimingLogger* timings, + bool clear_soft_references, + collector::GarbageCollector* collector) { + TimingLogger::ScopedTiming t(concurrent ? __FUNCTION__ : "(Paused)ProcessReferences", timings); + Thread* self = Thread::Current(); { MutexLock mu(self, *Locks::reference_processor_lock_); + collector_ = collector; if (!kUseReadBarrier) { - CHECK_EQ(SlowPathEnabled(), concurrent_) << "Slow path must be enabled iff concurrent"; + CHECK_EQ(SlowPathEnabled(), concurrent) << "Slow path must be enabled iff concurrent"; } else { - // Weak ref access is enabled at Zygote compaction by SemiSpace (concurrent_ == false). - CHECK_EQ(!self->GetWeakRefAccessEnabled(), concurrent_); + // Weak ref access is enabled at Zygote compaction by SemiSpace (concurrent == false). + CHECK_EQ(!self->GetWeakRefAccessEnabled(), concurrent); } - DCHECK(rp_state_ == RpState::kStarting); - rp_state_ = RpState::kInitMarkingDone; - condition_.Broadcast(self); } - if (kIsDebugBuild && collector_->IsTransactionActive()) { + if (kIsDebugBuild && collector->IsTransactionActive()) { // In transaction mode, we shouldn't enqueue any Reference to the queues. // See DelayReferenceReferent(). DCHECK(soft_reference_queue_.IsEmpty()); @@ -228,84 +200,75 @@ void ReferenceProcessor::ProcessReferences(Thread* self, TimingLogger* timings) DCHECK(finalizer_reference_queue_.IsEmpty()); DCHECK(phantom_reference_queue_.IsEmpty()); } - // Clear all remaining soft and weak references with white referents. - // This misses references only reachable through finalizers. - soft_reference_queue_.ClearWhiteReferences(&cleared_references_, collector_); - weak_reference_queue_.ClearWhiteReferences(&cleared_references_, collector_); - // Defer PhantomReference processing until we've finished marking through finalizers. - { - // TODO: Capture mark state of some system weaks here. If the referent was marked here, - // then it is now safe to return, since it can only refer to marked objects. If it becomes - // marked below, that is no longer guaranteed. - MutexLock mu(self, *Locks::reference_processor_lock_); - rp_state_ = RpState::kInitClearingDone; - // At this point, all mutator-accessible data is marked (black). Objects enqueued for - // finalization will only be made available to the mutator via CollectClearedReferences after - // we're fully done marking. Soft and WeakReferences accessible to the mutator have been - // processed and refer only to black objects. Thus there is no danger of the mutator getting - // access to non-black objects. Weak reference processing is still nominally suspended, - // But many kinds of references, including all java.lang.ref ones, are handled normally from - // here on. See GetReferent(). + // Unless required to clear soft references with white references, preserve some white referents. + if (!clear_soft_references) { + TimingLogger::ScopedTiming split(concurrent ? "ForwardSoftReferences" : + "(Paused)ForwardSoftReferences", timings); + if (concurrent) { + StartPreservingReferences(self); + } + // TODO: Add smarter logic for preserving soft references. The behavior should be a conditional + // mark if the SoftReference is supposed to be preserved. + uint32_t non_null_refs = soft_reference_queue_.ForwardSoftReferences(collector); + if (ATraceEnabled()) { + static constexpr size_t kBufSize = 80; + char buf[kBufSize]; + snprintf(buf, kBufSize, "Marking for %" PRIu32 " SoftReferences", non_null_refs); + ATraceBegin(buf); + collector->ProcessMarkStack(); + ATraceEnd(); + } else { + collector->ProcessMarkStack(); + } + if (concurrent) { + StopPreservingReferences(self); + } } + // Clear all remaining soft and weak references with white referents. + soft_reference_queue_.ClearWhiteReferences(&cleared_references_, collector); + weak_reference_queue_.ClearWhiteReferences(&cleared_references_, collector); { - TimingLogger::ScopedTiming t2( - concurrent_ ? "EnqueueFinalizerReferences" : "(Paused)EnqueueFinalizerReferences", timings); + TimingLogger::ScopedTiming t2(concurrent ? "EnqueueFinalizerReferences" : + "(Paused)EnqueueFinalizerReferences", timings); + if (concurrent) { + StartPreservingReferences(self); + } // Preserve all white objects with finalize methods and schedule them for finalization. FinalizerStats finalizer_stats = - finalizer_reference_queue_.EnqueueFinalizerReferences(&cleared_references_, collector_); + finalizer_reference_queue_.EnqueueFinalizerReferences(&cleared_references_, collector); if (ATraceEnabled()) { static constexpr size_t kBufSize = 80; char buf[kBufSize]; snprintf(buf, kBufSize, "Marking from %" PRIu32 " / %" PRIu32 " finalizers", finalizer_stats.num_enqueued_, finalizer_stats.num_refs_); ATraceBegin(buf); - collector_->ProcessMarkStack(); + collector->ProcessMarkStack(); ATraceEnd(); } else { - collector_->ProcessMarkStack(); + collector->ProcessMarkStack(); + } + if (concurrent) { + StopPreservingReferences(self); } } - - // Process all soft and weak references with white referents, where the references are reachable - // only from finalizers. It is unclear that there is any way to do this without slightly - // violating some language spec. We choose to apply normal Reference processing rules for these. - // This exposes the following issues: - // 1) In the case of an unmarked referent, we may end up enqueuing an "unreachable" reference. - // This appears unavoidable, since we need to clear the reference for safety, unless we - // mark the referent and undo finalization decisions for objects we encounter during marking. - // (Some versions of the RI seem to do something along these lines.) - // Or we could clear the reference without enqueuing it, which also seems strange and - // unhelpful. - // 2) In the case of a marked referent, we will preserve a reference to objects that may have - // been enqueued for finalization. Again fixing this would seem to involve at least undoing - // previous finalization / reference clearing decisions. (This would also mean than an object - // containing both a strong and a WeakReference to the same referent could see the - // WeakReference cleared.) - // The treatment in (2) is potentially quite dangerous, since Reference.get() can e.g. return a - // finalized object containing pointers to native objects that have already been deallocated. - // But it can be argued that this is just an instance of the broader rule that it is not safe - // for finalizers to access otherwise inaccessible finalizable objects. - soft_reference_queue_.ClearWhiteReferences(&cleared_references_, collector_, - /*report_cleared=*/ true); - weak_reference_queue_.ClearWhiteReferences(&cleared_references_, collector_, - /*report_cleared=*/ true); - - // Clear all phantom references with white referents. It's fine to do this just once here. - phantom_reference_queue_.ClearWhiteReferences(&cleared_references_, collector_); - + // Clear all finalizer referent reachable soft and weak references with white referents. + soft_reference_queue_.ClearWhiteReferences(&cleared_references_, collector); + weak_reference_queue_.ClearWhiteReferences(&cleared_references_, collector); + // Clear all phantom references with white referents. + phantom_reference_queue_.ClearWhiteReferences(&cleared_references_, collector); // At this point all reference queues other than the cleared references should be empty. DCHECK(soft_reference_queue_.IsEmpty()); DCHECK(weak_reference_queue_.IsEmpty()); DCHECK(finalizer_reference_queue_.IsEmpty()); DCHECK(phantom_reference_queue_.IsEmpty()); - { MutexLock mu(self, *Locks::reference_processor_lock_); // Need to always do this since the next GC may be concurrent. Doing this for only concurrent // could result in a stale is_marked_callback_ being called before the reference processing // starts since there is a small window of time where slow_path_enabled_ is enabled but the // callback isn't yet set. - if (!kUseReadBarrier && concurrent_) { + collector_ = nullptr; + if (!kUseReadBarrier && concurrent) { // Done processing, disable the slow path and broadcast to the waiters. DisableSlowPath(self); } diff --git a/runtime/gc/reference_processor.h b/runtime/gc/reference_processor.h index 0f84211a87..51c50544bf 100644 --- a/runtime/gc/reference_processor.h +++ b/runtime/gc/reference_processor.h @@ -46,29 +46,19 @@ class Heap; class ReferenceProcessor { public: ReferenceProcessor(); - - // Initialize for a reference processing pass. Called before suspending weak - // access. - void Setup(Thread* self, - collector::GarbageCollector* collector, - bool concurrent, - bool clear_soft_references) - REQUIRES(!Locks::reference_processor_lock_); - // Enqueue all types of java.lang.ref.References, and mark through finalizers. - // Assumes there is no concurrent mutator-driven marking, i.e. all potentially - // mutator-accessible objects should be marked before this. - void ProcessReferences(Thread* self, TimingLogger* timings) + void ProcessReferences(bool concurrent, + TimingLogger* timings, + bool clear_soft_references, + gc::collector::GarbageCollector* collector) REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_) REQUIRES(!Locks::reference_processor_lock_); - // The slow path bool is contained in the reference class object, can only be set once // Only allow setting this with mutators suspended so that we can avoid using a lock in the // GetReferent fast path as an optimization. void EnableSlowPath() REQUIRES_SHARED(Locks::mutator_lock_); void BroadcastForSlowPath(Thread* self); - // Decode the referent, may block if references are being processed. In the normal - // no-read-barrier or Baker-read-barrier cases, we assume reference is not a PhantomReference. + // Decode the referent, may block if references are being processed. ObjPtr<mirror::Object> GetReferent(Thread* self, ObjPtr<mirror::Reference> reference) REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!Locks::reference_processor_lock_); // Collects the cleared references and returns a task, to be executed after FinishGC, that will @@ -88,30 +78,27 @@ class ReferenceProcessor { void ClearReferent(ObjPtr<mirror::Reference> ref) REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!Locks::reference_processor_lock_); - uint32_t ForwardSoftReferences(TimingLogger* timings) - REQUIRES_SHARED(Locks::mutator_lock_); private: bool SlowPathEnabled() REQUIRES_SHARED(Locks::mutator_lock_); // Called by ProcessReferences. void DisableSlowPath(Thread* self) REQUIRES(Locks::reference_processor_lock_) REQUIRES_SHARED(Locks::mutator_lock_); + // If we are preserving references it means that some dead objects may become live, we use start + // and stop preserving to block mutators using GetReferrent from getting access to these + // referents. + void StartPreservingReferences(Thread* self) REQUIRES(!Locks::reference_processor_lock_); + void StopPreservingReferences(Thread* self) REQUIRES(!Locks::reference_processor_lock_); // Wait until reference processing is done. void WaitUntilDoneProcessingReferences(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::reference_processor_lock_); // Collector which is clearing references, used by the GetReferent to return referents which are - // already marked. Only updated by thread currently running GC. - // Guarded by reference_processor_lock_ when not read by collector. Only the collector changes - // it. - collector::GarbageCollector* collector_; - // Reference processor state. Only valid while weak reference processing is suspended. - // Used by GetReferent and friends to return early. - enum class RpState : uint8_t { kStarting, kInitMarkingDone, kInitClearingDone }; - RpState rp_state_ GUARDED_BY(Locks::reference_processor_lock_); - bool concurrent_; // Running concurrently with mutator? Only used by GC thread. - bool clear_soft_references_; // Only used by GC thread. - + // already marked. + collector::GarbageCollector* collector_ GUARDED_BY(Locks::reference_processor_lock_); + // Boolean for whether or not we are preserving references (either soft references or finalizers). + // If this is true, then we cannot return a referent (see comment in GetReferent). + bool preserving_references_ GUARDED_BY(Locks::reference_processor_lock_); // Condition that people wait on if they attempt to get the referent of a reference while // processing is in progress. Broadcast when an empty checkpoint is requested, but not for other // checkpoints or thread suspensions. See mutator_gc_coord.md. diff --git a/runtime/gc/reference_queue.cc b/runtime/gc/reference_queue.cc index 6bdacaf18c..568ca04c1d 100644 --- a/runtime/gc/reference_queue.cc +++ b/runtime/gc/reference_queue.cc @@ -131,8 +131,7 @@ size_t ReferenceQueue::GetLength() const { } void ReferenceQueue::ClearWhiteReferences(ReferenceQueue* cleared_references, - collector::GarbageCollector* collector, - bool report_cleared) { + collector::GarbageCollector* collector) { while (!IsEmpty()) { ObjPtr<mirror::Reference> ref = DequeuePendingReference(); mirror::HeapReference<mirror::Object>* referent_addr = ref->GetReferentReferenceAddr(); @@ -146,15 +145,6 @@ void ReferenceQueue::ClearWhiteReferences(ReferenceQueue* cleared_references, ref->ClearReferent<false>(); } cleared_references->EnqueueReference(ref); - if (report_cleared) { - static bool already_reported = false; - if (!already_reported) { - // TODO: Maybe do this only if the queue is non-null? - LOG(WARNING) - << "Cleared Reference was only reachable from finalizer (only reported once)"; - already_reported = true; - } - } } // Delay disabling the read barrier until here so that the ClearReferent call above in // transaction mode will trigger the read barrier. @@ -192,33 +182,22 @@ FinalizerStats ReferenceQueue::EnqueueFinalizerReferences(ReferenceQueue* cleare } uint32_t ReferenceQueue::ForwardSoftReferences(MarkObjectVisitor* visitor) { + if (UNLIKELY(IsEmpty())) { + return 0; + } uint32_t num_refs(0); - Thread* self = Thread::Current(); - static constexpr int SR_BUF_SIZE = 32; - ObjPtr<mirror::Reference> buf[SR_BUF_SIZE]; - int n_entries; - bool empty; + const ObjPtr<mirror::Reference> head = list_; + ObjPtr<mirror::Reference> ref = head; do { - { - // Acquire lock only a few times and hold it as briefly as possible. - MutexLock mu(self, *lock_); - empty = IsEmpty(); - for (n_entries = 0; n_entries < SR_BUF_SIZE && !empty; ++n_entries) { - // Dequeuing the Reference here means it could possibly be enqueued again during this GC. - // That's unlikely and benign. - buf[n_entries] = DequeuePendingReference(); - empty = IsEmpty(); - } - } - for (int i = 0; i < n_entries; ++i) { - mirror::HeapReference<mirror::Object>* referent_addr = buf[i]->GetReferentReferenceAddr(); - if (referent_addr->AsMirrorPtr() != nullptr) { - visitor->MarkHeapReference(referent_addr, /*do_atomic_update=*/ true); - ++num_refs; - } - DisableReadBarrierForReference(buf[i]->AsReference()); + mirror::HeapReference<mirror::Object>* referent_addr = ref->GetReferentReferenceAddr(); + if (referent_addr->AsMirrorPtr() != nullptr) { + // do_atomic_update is false because mutators can't access the referent due to the weak ref + // access blocking. + visitor->MarkHeapReference(referent_addr, /*do_atomic_update=*/ false); + ++num_refs; } - } while (!empty); + ref = ref->GetPendingNext(); + } while (LIKELY(ref != head)); return num_refs; } diff --git a/runtime/gc/reference_queue.h b/runtime/gc/reference_queue.h index 3fda7167d4..06243c40a2 100644 --- a/runtime/gc/reference_queue.h +++ b/runtime/gc/reference_queue.h @@ -90,20 +90,17 @@ class ReferenceQueue { collector::GarbageCollector* collector) REQUIRES_SHARED(Locks::mutator_lock_); - // Walks the reference list marking and dequeuing any references subject to the reference - // clearing policy. References with a black referent are removed from the list. References - // with white referents biased toward saving are blackened and also removed from the list. - // Returns the number of non-null soft references. May be called concurrently with - // AtomicEnqueueIfNotEnqueued(). + // Walks the reference list marking any references subject to the reference clearing policy. + // References with a black referent are removed from the list. References with white referents + // biased toward saving are blackened and also removed from the list. + // Returns the number of non-null soft references. uint32_t ForwardSoftReferences(MarkObjectVisitor* visitor) - REQUIRES(!*lock_) REQUIRES_SHARED(Locks::mutator_lock_); // Unlink the reference list clearing references objects with white referents. Cleared references // registered to a reference queue are scheduled for appending by the heap worker thread. void ClearWhiteReferences(ReferenceQueue* cleared_references, - collector::GarbageCollector* collector, - bool report_cleared = false) + collector::GarbageCollector* collector) REQUIRES_SHARED(Locks::mutator_lock_); void Dump(std::ostream& os) const REQUIRES_SHARED(Locks::mutator_lock_); @@ -112,12 +109,9 @@ class ReferenceQueue { bool IsEmpty() const { return list_ == nullptr; } - - // Clear this queue. Only safe after handing off the contents elsewhere for further processing. void Clear() { list_ = nullptr; } - mirror::Reference* GetList() REQUIRES_SHARED(Locks::mutator_lock_) { return list_; } @@ -130,10 +124,8 @@ class ReferenceQueue { // Lock, used for parallel GC reference enqueuing. It allows for multiple threads simultaneously // calling AtomicEnqueueIfNotEnqueued. Mutex* const lock_; - // The actual reference list. Only a root for the mark compact GC since it - // will be null during root marking for other GC types. Not an ObjPtr since it - // is accessed from multiple threads. Points to a singly-linked circular list - // using the pendingNext field. + // The actual reference list. Only a root for the mark compact GC since it will be null for other + // GC types. Not an ObjPtr since it is accessed from multiple threads. mirror::Reference* list_; DISALLOW_IMPLICIT_CONSTRUCTORS(ReferenceQueue); diff --git a/runtime/intern_table.cc b/runtime/intern_table.cc index 5d4c50727e..25c45cba6a 100644 --- a/runtime/intern_table.cc +++ b/runtime/intern_table.cc @@ -32,7 +32,6 @@ #include "object_callbacks.h" #include "scoped_thread_state_change-inl.h" #include "thread.h" -#include "thread-inl.h" namespace art { diff --git a/runtime/intern_table.h b/runtime/intern_table.h index 9d1720ffa5..db6016eba7 100644 --- a/runtime/intern_table.h +++ b/runtime/intern_table.h @@ -116,7 +116,7 @@ class InternTable { REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!Roles::uninterruptible_); // Only used by image writer. Special version that may not cause thread suspension since the GC - // cannot be running while we are doing image writing. Maybe be called while holding a + // cannot be running while we are doing image writing. Maybe be called while while holding a // lock since there will not be thread suspension. ObjPtr<mirror::String> InternStrongImageString(ObjPtr<mirror::String> s) REQUIRES_SHARED(Locks::mutator_lock_); diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h index e9b78f3259..1809227f2c 100644 --- a/runtime/interpreter/interpreter_common.h +++ b/runtime/interpreter/interpreter_common.h @@ -59,7 +59,6 @@ #include "obj_ptr.h" #include "stack.h" #include "thread.h" -#include "thread-inl.h" #include "unstarted_runtime.h" #include "verifier/method_verifier.h" #include "well_known_classes.h" diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc index ad44ac2094..34ddfc4233 100644 --- a/runtime/jit/jit_code_cache.cc +++ b/runtime/jit/jit_code_cache.cc @@ -57,7 +57,6 @@ #include "scoped_thread_state_change-inl.h" #include "stack.h" #include "thread-current-inl.h" -#include "thread-inl.h" #include "thread_list.h" namespace art { diff --git a/runtime/jni/java_vm_ext.cc b/runtime/jni/java_vm_ext.cc index 94108c69de..12e6d22252 100644 --- a/runtime/jni/java_vm_ext.cc +++ b/runtime/jni/java_vm_ext.cc @@ -864,10 +864,6 @@ ObjPtr<mirror::Object> JavaVMExt::DecodeWeakGlobalLocked(Thread* self, IndirectR if (kDebugLocking) { Locks::jni_weak_globals_lock_->AssertHeld(self); } - // TODO: Handle the already null case without waiting. - // TODO: Otherwise we should just wait for kInitMarkingDone, and track which weak globals were - // marked at that point. We would only need one mark bit per entry in the weak_globals_ table, - // and a quick pass over that early on during reference processing. WaitForWeakGlobalsAccess(self); return weak_globals_.Get(ref); } diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h index c0dcf8846b..3c1e7a0a00 100644 --- a/runtime/thread-inl.h +++ b/runtime/thread-inl.h @@ -372,25 +372,6 @@ inline bool Thread::PushOnThreadLocalAllocationStack(mirror::Object* obj) { return false; } -inline bool Thread::GetWeakRefAccessEnabled() const { - CHECK(kUseReadBarrier); - DCHECK(this == Thread::Current()); - WeakRefAccessState s = tls32_.weak_ref_access_enabled.load(std::memory_order_acquire); - if (s == WeakRefAccessState::kVisiblyEnabled) { - return true; - } else if (s == WeakRefAccessState::kDisabled) { - return false; - } - DCHECK(s == WeakRefAccessState::kEnabled) - << "state = " << static_cast<std::underlying_type_t<WeakRefAccessState>>(s); - // The state is only changed back to DISABLED during a checkpoint. Thus no other thread can - // change the value concurrently here. No other thread reads the value we store here, so there - // is no need for a release store. - tls32_.weak_ref_access_enabled.store(WeakRefAccessState::kVisiblyEnabled, - std::memory_order_relaxed); - return true; -} - inline void Thread::SetThreadLocalAllocationStack(StackReference<mirror::Object>* start, StackReference<mirror::Object>* end) { DCHECK(Thread::Current() == this) << "Should be called by self"; diff --git a/runtime/thread.h b/runtime/thread.h index 7eb57e914c..1085a563c4 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -171,20 +171,6 @@ enum class DeoptimizationMethodType { kDefault // dex pc may or may not advance depending on other conditions. }; -// For the CC colector, normal weak reference access can be disabled on a per-thread basis, while -// processing references. After finishing, the reference processor asynchronously sets the -// per-thread flags back to kEnabled with release memory ordering semantics. Each mutator thread -// should check its flag with acquire semantics before assuming that it is enabled. However, -// that is often too expensive, so the reading thread sets it to kVisiblyEnabled after seeing it -// kEnabled. The Reference.get() intrinsic can thus read it in relaxed mode, and reread (by -// resorting to the slow path) with acquire semantics if it sees a value of kEnabled rather than -// kVisiblyEnabled. -enum class WeakRefAccessState : int32_t { - kVisiblyEnabled = 0, // Enabled, and previously read with acquire load by this thread. - kEnabled, - kDisabled -}; - // This should match RosAlloc::kNumThreadLocalSizeBrackets. static constexpr size_t kNumRosAllocThreadLocalSizeBracketsInThread = 16; @@ -1008,13 +994,14 @@ class Thread { void SetIsGcMarkingAndUpdateEntrypoints(bool is_marking); - bool GetWeakRefAccessEnabled() const; // Only safe for current thread. + bool GetWeakRefAccessEnabled() const { + CHECK(kUseReadBarrier); + return tls32_.weak_ref_access_enabled; + } void SetWeakRefAccessEnabled(bool enabled) { CHECK(kUseReadBarrier); - WeakRefAccessState new_state = enabled ? - WeakRefAccessState::kEnabled : WeakRefAccessState::kDisabled; - tls32_.weak_ref_access_enabled.store(new_state, std::memory_order_release); + tls32_.weak_ref_access_enabled = enabled; } uint32_t GetDisableThreadFlipCount() const { @@ -1685,7 +1672,7 @@ class Thread { thread_exit_check_count(0), is_transitioning_to_runnable(false), is_gc_marking(false), - weak_ref_access_enabled(WeakRefAccessState::kVisiblyEnabled), + weak_ref_access_enabled(true), disable_thread_flip_count(0), user_code_suspend_count(0), force_interpreter_count(0), @@ -1741,17 +1728,14 @@ class Thread { AtomicInteger park_state_; - // Determines whether the thread is allowed to directly access a weak ref - // (Reference::GetReferent() and system weaks) and to potentially mark an object alive/gray. - // This is used for concurrent reference processing of the CC collector only. This is thread - // local so that we can enable/disable weak ref access by using a checkpoint and avoid a race - // around the time weak ref access gets disabled and concurrent reference processing begins - // (if weak ref access is disabled during a pause, this is not an issue.) Other collectors use - // Runtime::DisallowNewSystemWeaks() and ReferenceProcessor::EnableSlowPath(). Can be - // concurrently accessed by GetReferent() and set (by iterating over threads). - // Can be changed from kEnabled to kVisiblyEnabled by readers. No other concurrent access is - // possible when that happens. - mutable std::atomic<WeakRefAccessState> weak_ref_access_enabled; + // True if the thread is allowed to access a weak ref (Reference::GetReferent() and system + // weaks) and to potentially mark an object alive/gray. This is used for concurrent reference + // processing of the CC collector only. This is thread local so that we can enable/disable weak + // ref access by using a checkpoint and avoid a race around the time weak ref access gets + // disabled and concurrent reference processing begins (if weak ref access is disabled during a + // pause, this is not an issue.) Other collectors use Runtime::DisallowNewSystemWeaks() and + // ReferenceProcessor::EnableSlowPath(). + bool32_t weak_ref_access_enabled; // A thread local version of Heap::disable_thread_flip_count_. This keeps track of how many // levels of (nested) JNI critical sections the thread is in and is used to detect a nested JNI |