diff --git a/runtime/vm/heap/gc_shared.h b/runtime/vm/heap/gc_shared.h index 378932b5676..7f4a26f2932 100644 --- a/runtime/vm/heap/gc_shared.h +++ b/runtime/vm/heap/gc_shared.h @@ -159,119 +159,111 @@ void RunNativeFinalizerCallback(NativeFinalizerPtr raw_finalizer, // fields referring to dead objects and |kName| field which contains visitor // name for tracing output. template -void MournFinalized(GCVisitorType* visitor) { - FinalizerEntryPtr current_entry = - visitor->delayed_.finalizer_entries.Release(); - while (current_entry != FinalizerEntry::null()) { - TRACE_FINALIZER("Processing Entry %p", current_entry->untag()); - FinalizerEntryPtr next_entry = - current_entry->untag()->next_seen_by_gc_.Decompress( - current_entry->heap_base()); - current_entry->untag()->next_seen_by_gc_ = FinalizerEntry::null(); +void MournFinalizerEntry(GCVisitorType* visitor, + FinalizerEntryPtr current_entry) { + TRACE_FINALIZER("Processing Entry %p", current_entry->untag()); - uword heap_base = current_entry->heap_base(); - const Heap::Space before_gc_space = SpaceForExternal(current_entry); - const bool value_collected_this_gc = - GCVisitorType::ForwardOrSetNullIfCollected( - heap_base, ¤t_entry->untag()->value_); - if (!value_collected_this_gc && before_gc_space == Heap::kNew) { - const Heap::Space after_gc_space = SpaceForExternal(current_entry); - if (after_gc_space == Heap::kOld) { - const intptr_t external_size = current_entry->untag()->external_size_; - TRACE_FINALIZER("Promoting external size %" Pd - " bytes from new to old space", - external_size); - visitor->isolate_group()->heap()->PromotedExternal(external_size); - } + uword heap_base = current_entry->heap_base(); + const Heap::Space before_gc_space = SpaceForExternal(current_entry); + const bool value_collected_this_gc = + GCVisitorType::ForwardOrSetNullIfCollected( + heap_base, ¤t_entry->untag()->value_); + if (!value_collected_this_gc && before_gc_space == Heap::kNew) { + const Heap::Space after_gc_space = SpaceForExternal(current_entry); + if (after_gc_space == Heap::kOld) { + const intptr_t external_size = current_entry->untag()->external_size_; + TRACE_FINALIZER("Promoting external size %" Pd + " bytes from new to old space", + external_size); + visitor->isolate_group()->heap()->PromotedExternal(external_size); } - GCVisitorType::ForwardOrSetNullIfCollected( - heap_base, ¤t_entry->untag()->detach_); - GCVisitorType::ForwardOrSetNullIfCollected( - heap_base, ¤t_entry->untag()->finalizer_); + } + GCVisitorType::ForwardOrSetNullIfCollected(heap_base, + ¤t_entry->untag()->detach_); + GCVisitorType::ForwardOrSetNullIfCollected( + heap_base, ¤t_entry->untag()->finalizer_); - ObjectPtr token_object = current_entry->untag()->token(); - // See sdk/lib/_internal/vm/lib/internal_patch.dart FinalizerBase.detach. - const bool is_detached = token_object == current_entry; + ObjectPtr token_object = current_entry->untag()->token(); + // See sdk/lib/_internal/vm/lib/internal_patch.dart FinalizerBase.detach. + const bool is_detached = token_object == current_entry; - if (value_collected_this_gc && !is_detached) { - FinalizerBasePtr finalizer = current_entry->untag()->finalizer(); + if (!value_collected_this_gc) return; + if (is_detached) return; - if (finalizer.IsRawNull()) { - TRACE_FINALIZER("Value collected entry %p finalizer null", - current_entry->untag()); + FinalizerBasePtr finalizer = current_entry->untag()->finalizer(); - // Do nothing, the finalizer has been GCed. - } else { - TRACE_FINALIZER("Value collected entry %p finalizer %p", - current_entry->untag(), finalizer->untag()); + if (finalizer.IsRawNull()) { + TRACE_FINALIZER("Value collected entry %p finalizer null", + current_entry->untag()); - FinalizerPtr finalizer_dart = static_cast(finalizer); - // Move entry to entries collected and current head of that list as - // the next element. Using a atomic exchange satisfies concurrency - // between the parallel GC tasks. - // We rely on the fact that the mutator thread is not running to avoid - // races between GC and mutator modifying Finalizer.entries_collected. - // - // We only run in serial marker or in the finalize step in the marker, - // both are in safepoint. - // The main scavenger worker is at safepoint, the other scavenger - // workers are not, but they bypass safepoint because the main - // worker is at a safepoint already. - ASSERT(Thread::Current()->OwnsGCSafepoint() || - Thread::Current()->BypassSafepoints()); + // Do nothing, the finalizer has been GCed. + return; + } - if (finalizer.IsNativeFinalizer()) { - NativeFinalizerPtr native_finalizer = - static_cast(finalizer); + TRACE_FINALIZER("Value collected entry %p finalizer %p", + current_entry->untag(), finalizer->untag()); - // Immediately call native callback. - RunNativeFinalizerCallback(native_finalizer, current_entry, - before_gc_space, visitor); + FinalizerPtr finalizer_dart = static_cast(finalizer); + // Move entry to entries collected and current head of that list as + // the next element. Using a atomic exchange satisfies concurrency + // between the parallel GC tasks. + // We rely on the fact that the mutator thread is not running to avoid + // races between GC and mutator modifying Finalizer.entries_collected. + // + // We only run in serial marker or in the finalize step in the marker, + // both are in safepoint. + // The main scavenger worker is at safepoint, the other scavenger + // workers are not, but they bypass safepoint because the main + // worker is at a safepoint already. + ASSERT(Thread::Current()->OwnsGCSafepoint() || + Thread::Current()->BypassSafepoints()); - // Fall-through sending a message to clear the entries and remove - // from detachments. - } + if (finalizer.IsNativeFinalizer()) { + NativeFinalizerPtr native_finalizer = + static_cast(finalizer); - FinalizerEntryPtr previous_head = - finalizer_dart->untag()->exchange_entries_collected(current_entry); - current_entry->untag()->set_next(previous_head); - const bool first_entry = previous_head.IsRawNull(); + // Immediately call native callback. + RunNativeFinalizerCallback(native_finalizer, current_entry, before_gc_space, + visitor); - // If we're in the marker, we need to ensure that we release the store - // buffer afterwards. - // If we're in the scavenger and have the finalizer in old space and - // a new space entry, we don't need to release the store buffer. - if (!first_entry && previous_head->IsNewObject() && - current_entry->IsOldObject()) { - TRACE_FINALIZER("Entry %p (old) next is %p (new)", - current_entry->untag(), previous_head->untag()); - // We must release the thread's store buffer block. - } + // Fall-through sending a message to clear the entries and remove + // from detachments. + } - // Schedule calling Dart finalizer. - if (first_entry) { - Isolate* isolate = finalizer->untag()->isolate_; - if (isolate == nullptr) { - TRACE_FINALIZER( - "Not scheduling finalizer %p callback on isolate null", - finalizer->untag()); - } else { - TRACE_FINALIZER("Scheduling finalizer %p callback on isolate %p", - finalizer->untag(), isolate); + FinalizerEntryPtr previous_head = + finalizer_dart->untag()->exchange_entries_collected(current_entry); + current_entry->untag()->set_next(previous_head); + const bool first_entry = previous_head.IsRawNull(); - PersistentHandle* handle = - isolate->group()->api_state()->AllocatePersistentHandle(); - handle->set_ptr(finalizer); - MessageHandler* message_handler = isolate->message_handler(); - message_handler->PostMessage( - Message::New(handle, Message::kNormalPriority), - /*before_events*/ false); - } - } - } + // If we're in the marker, we need to ensure that we release the store + // buffer afterwards. + // If we're in the scavenger and have the finalizer in old space and + // a new space entry, we don't need to release the store buffer. + if (!first_entry && previous_head->IsNewObject() && + current_entry->IsOldObject()) { + TRACE_FINALIZER("Entry %p (old) next is %p (new)", current_entry->untag(), + previous_head->untag()); + // We must release the thread's store buffer block. + } + + // Schedule calling Dart finalizer. + if (first_entry) { + Isolate* isolate = finalizer->untag()->isolate_; + if (isolate == nullptr) { + TRACE_FINALIZER("Not scheduling finalizer %p callback on isolate null", + finalizer->untag()); + } else { + TRACE_FINALIZER("Scheduling finalizer %p callback on isolate %p", + finalizer->untag(), isolate); + + PersistentHandle* handle = + isolate->group()->api_state()->AllocatePersistentHandle(); + handle->set_ptr(finalizer); + MessageHandler* message_handler = isolate->message_handler(); + message_handler->PostMessage( + Message::New(handle, Message::kNormalPriority), + /*before_events*/ false); } - - current_entry = next_entry; } } diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc index 122747e6729..ae22911cae2 100644 --- a/runtime/vm/heap/marker.cc +++ b/runtime/vm/heap/marker.cc @@ -267,8 +267,8 @@ class MarkingVisitorBase : public ObjectPointerVisitor { void FinalizeMarking() { work_list_.Finalize(); deferred_work_list_.Finalize(); - MournFinalized(this); - // MournFinalized inserts newly discovered dead entries into the + MournFinalizerEntries(); + // MournFinalizerEntries inserts newly discovered dead entries into the // linked list attached to the Finalizer. This might create // cross-generational references which might be added to the store // buffer. Release the store buffer to satisfy the invariant that @@ -278,47 +278,48 @@ class MarkingVisitorBase : public ObjectPointerVisitor { } void MournWeakProperties() { - WeakPropertyPtr cur_weak = delayed_.weak_properties.Release(); - while (cur_weak != WeakProperty::null()) { - WeakPropertyPtr next_weak = - cur_weak->untag()->next_seen_by_gc_.Decompress(cur_weak->heap_base()); - cur_weak->untag()->next_seen_by_gc_ = WeakProperty::null(); - RELEASE_ASSERT(!cur_weak->untag()->key()->untag()->IsMarked()); - WeakProperty::Clear(cur_weak); - cur_weak = next_weak; + WeakPropertyPtr current = delayed_.weak_properties.Release(); + while (current != WeakProperty::null()) { + WeakPropertyPtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = WeakProperty::null(); + current->untag()->key_ = Object::null(); + current->untag()->value_ = Object::null(); + current = next; } } void MournWeakReferences() { - WeakReferencePtr cur_weak = delayed_.weak_references.Release(); - while (cur_weak != WeakReference::null()) { - WeakReferencePtr next_weak = - cur_weak->untag()->next_seen_by_gc_.Decompress(cur_weak->heap_base()); - cur_weak->untag()->next_seen_by_gc_ = WeakReference::null(); - - // If we did not mark the target through a weak property in a later round, - // then the target is dead and we should clear it. - ForwardOrSetNullIfCollected(cur_weak->heap_base(), - &cur_weak->untag()->target_); - - cur_weak = next_weak; + WeakReferencePtr current = delayed_.weak_references.Release(); + while (current != WeakReference::null()) { + WeakReferencePtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = WeakReference::null(); + ForwardOrSetNullIfCollected(current->heap_base(), + ¤t->untag()->target_); + current = next; } } void MournWeakArrays() { - WeakArrayPtr cur_weak = delayed_.weak_arrays.Release(); - while (cur_weak != WeakArray::null()) { - WeakArrayPtr next_weak = - cur_weak->untag()->next_seen_by_gc_.Decompress(cur_weak->heap_base()); - cur_weak->untag()->next_seen_by_gc_ = WeakArray::null(); - - intptr_t length = Smi::Value(cur_weak->untag()->length()); + WeakArrayPtr current = delayed_.weak_arrays.Release(); + while (current != WeakArray::null()) { + WeakArrayPtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = WeakArray::null(); + intptr_t length = Smi::Value(current->untag()->length()); for (intptr_t i = 0; i < length; i++) { - ForwardOrSetNullIfCollected(cur_weak->heap_base(), - &cur_weak->untag()->data()[i]); + ForwardOrSetNullIfCollected(current->heap_base(), + ¤t->untag()->data()[i]); } + current = next; + } + } - cur_weak = next_weak; + void MournFinalizerEntries() { + FinalizerEntryPtr current = delayed_.finalizer_entries.Release(); + while (current != FinalizerEntry::null()) { + FinalizerEntryPtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = FinalizerEntry::null(); + MournFinalizerEntry(this, current); + current = next; } } @@ -438,9 +439,6 @@ class MarkingVisitorBase : public ObjectPointerVisitor { uintptr_t marked_bytes_; int64_t marked_micros_; - template - friend void MournFinalized(GCVisitorType* visitor); - DISALLOW_IMPLICIT_CONSTRUCTORS(MarkingVisitorBase); }; @@ -761,8 +759,8 @@ class ParallelMarkTask : public ThreadPool::Task { visitor_->MournWeakProperties(); visitor_->MournWeakReferences(); visitor_->MournWeakArrays(); - // Don't MournFinalized here, do it on main thread, so that we don't have - // to coordinate workers. + // Don't MournFinalizerEntries here, do it on main thread, so that we + // don't have to coordinate workers. marker_->IterateWeakRoots(thread); int64_t stop = OS::GetCurrentMonotonicMicros(); @@ -1066,7 +1064,7 @@ void GCMarker::MarkObjects(PageSpace* page_space) { visitor.MournWeakProperties(); visitor.MournWeakReferences(); visitor.MournWeakArrays(); - MournFinalized(&visitor); + visitor.MournFinalizerEntries(); IterateWeakRoots(thread); // All marking done; detach code, etc. int64_t stop = OS::GetCurrentMonotonicMicros(); diff --git a/runtime/vm/heap/scavenger.cc b/runtime/vm/heap/scavenger.cc index 8e0223092de..545da00ab46 100644 --- a/runtime/vm/heap/scavenger.cc +++ b/runtime/vm/heap/scavenger.cc @@ -321,7 +321,7 @@ class ScavengerVisitorBase : public ObjectPointerVisitor { MournWeakProperties(); MournWeakReferences(); MournWeakArrays(); - MournFinalized(this); + MournFinalizerEntries(); } page_space_->ReleaseLock(freelist_); thread_ = nullptr; @@ -555,9 +555,51 @@ class ScavengerVisitorBase : public ObjectPointerVisitor { return !IsForwarding(ReadHeaderRelaxed(raw)); } - void MournWeakProperties(); - void MournWeakReferences(); - void MournWeakArrays(); + void MournWeakProperties() { + WeakPropertyPtr current = delayed_.weak_properties.Release(); + while (current != WeakProperty::null()) { + WeakPropertyPtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = WeakProperty::null(); + current->untag()->key_ = Object::null(); + current->untag()->value_ = Object::null(); + current = next; + } + } + + void MournWeakReferences() { + WeakReferencePtr current = delayed_.weak_references.Release(); + while (current != WeakReference::null()) { + WeakReferencePtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = WeakReference::null(); + ForwardOrSetNullIfCollected(current->heap_base(), + ¤t->untag()->target_); + current = next; + } + } + + void MournWeakArrays() { + WeakArrayPtr current = delayed_.weak_arrays.Release(); + while (current != WeakArray::null()) { + WeakArrayPtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = WeakArray::null(); + intptr_t length = Smi::Value(current->untag()->length()); + for (intptr_t i = 0; i < length; i++) { + ForwardOrSetNullIfCollected(current->heap_base(), + &(current->untag()->data()[i])); + } + current = next; + } + } + + void MournFinalizerEntries() { + FinalizerEntryPtr current = delayed_.finalizer_entries.Release(); + while (current != FinalizerEntry::null()) { + FinalizerEntryPtr next = current->untag()->next_seen_by_gc(); + current->untag()->next_seen_by_gc_ = FinalizerEntry::null(); + MournFinalizerEntry(this, current); + current = next; + } + } Thread* thread_; Scavenger* scavenger_; @@ -573,9 +615,6 @@ class ScavengerVisitorBase : public ObjectPointerVisitor { Page* tail_ = nullptr; // Allocating from here. Page* scan_ = nullptr; // Resolving from here. - template - friend void MournFinalized(GCVisitorType* visitor); - DISALLOW_COPY_AND_ASSIGN(ScavengerVisitorBase); }; @@ -1415,7 +1454,7 @@ void ScavengerVisitorBase::ProcessOldFinalizerEntry( // keys, and finalizers in new space won't be reclaimed until after they // are promoted. // This will only visit the strong references, end enqueue the entry. - // This enables us to update external space in MournFinalized. + // This enables us to update external space in MournFinalizerEntries. const Heap::Space before_gc_space = SpaceForExternal(raw_entry); UntaggedFinalizerEntry::VisitFinalizerEntryPointers(raw_entry, this); if (before_gc_space == Heap::kNew) { @@ -1501,79 +1540,6 @@ void Scavenger::MournWeakTables() { /*at_safepoint=*/true); } -template -void ScavengerVisitorBase::MournWeakProperties() { - ASSERT(!scavenger_->abort_); - - // The queued weak properties at this point do not refer to reachable keys, - // so we clear their key and value fields. - WeakPropertyPtr cur_weak = delayed_.weak_properties.Release(); - while (cur_weak != WeakProperty::null()) { - WeakPropertyPtr next_weak = - cur_weak->untag()->next_seen_by_gc_.Decompress(cur_weak->heap_base()); - // Reset the next pointer in the weak property. - cur_weak->untag()->next_seen_by_gc_ = WeakProperty::null(); - -#if defined(DEBUG) - ObjectPtr raw_key = cur_weak->untag()->key(); - uword raw_addr = UntaggedObject::ToAddr(raw_key); - uword header = *reinterpret_cast(raw_addr); - ASSERT(!IsForwarding(header)); - ASSERT(raw_key->IsHeapObject()); - ASSERT(raw_key->IsNewObject()); // Key still points into from space. -#endif // defined(DEBUG) - - WeakProperty::Clear(cur_weak); - - // Advance to next weak property in the queue. - cur_weak = next_weak; - } -} - -template -void ScavengerVisitorBase::MournWeakReferences() { - ASSERT(!scavenger_->abort_); - - // The queued weak references at this point either should have their target - // updated or should be cleared. - WeakReferencePtr cur_weak = delayed_.weak_references.Release(); - while (cur_weak != WeakReference::null()) { - WeakReferencePtr next_weak = - cur_weak->untag()->next_seen_by_gc_.Decompress(cur_weak->heap_base()); - // Reset the next pointer in the weak reference. - cur_weak->untag()->next_seen_by_gc_ = WeakReference::null(); - - // If we did not mark the target through a weak property in a later round, - // then the target is dead and we should clear it. - ForwardOrSetNullIfCollected(cur_weak->heap_base(), - &cur_weak->untag()->target_); - - // Advance to next weak reference in the queue. - cur_weak = next_weak; - } -} - -template -void ScavengerVisitorBase::MournWeakArrays() { - ASSERT(!scavenger_->abort_); - WeakArrayPtr cur_weak = delayed_.weak_arrays.Release(); - while (cur_weak != WeakArray::null()) { - WeakArrayPtr next_weak = - cur_weak->untag()->next_seen_by_gc_.Decompress(cur_weak->heap_base()); - // Reset the next pointer in the weak reference. - cur_weak->untag()->next_seen_by_gc_ = WeakArray::null(); - - intptr_t length = Smi::Value(cur_weak->untag()->length()); - for (intptr_t i = 0; i < length; i++) { - ForwardOrSetNullIfCollected(cur_weak->heap_base(), - &(cur_weak->untag()->data()[i])); - } - - // Advance to next weak reference in the queue. - cur_weak = next_weak; - } -} - // Returns whether the object referred to in `ptr_address` was GCed this GC. template bool ScavengerVisitorBase::ForwardOrSetNullIfCollected( diff --git a/runtime/vm/message_handler.h b/runtime/vm/message_handler.h index 62ccb02266b..54c56befb42 100644 --- a/runtime/vm/message_handler.h +++ b/runtime/vm/message_handler.h @@ -215,7 +215,7 @@ class MessageHandler { private: template - friend void MournFinalized(GCVisitorType* visitor); + friend void MournFinalizerEntry(GCVisitorType*, FinalizerEntryPtr); friend class PortMap; friend class MessageHandlerTestPeer; friend class MessageHandlerTask; diff --git a/runtime/vm/object.h b/runtime/vm/object.h index e3a11b9b719..59a14ca262c 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -12820,14 +12820,6 @@ class WeakProperty : public Instance { return RoundedAllocationSize(sizeof(UntaggedWeakProperty)); } - static void Clear(WeakPropertyPtr raw_weak) { - ASSERT(raw_weak->untag()->next_seen_by_gc_ == - CompressedWeakPropertyPtr(WeakProperty::null())); - // This action is performed by the GC. No barrier. - raw_weak->untag()->key_ = Object::null(); - raw_weak->untag()->value_ = Object::null(); - } - private: FINAL_HEAP_OBJECT_IMPLEMENTATION(WeakProperty, Instance); friend class Class; diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index 79ceb6de4cb..34e48463bb6 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -3521,7 +3521,7 @@ class UntaggedFinalizerBase : public UntaggedInstance { COMPRESSED_POINTER_FIELD(FinalizerEntryPtr, entries_collected) template - friend void MournFinalized(GCVisitorType* visitor); + friend void MournFinalizerEntry(GCVisitorType*, FinalizerEntryPtr); friend class GCMarker; template friend class MarkingVisitorBase; @@ -3546,7 +3546,7 @@ class UntaggedFinalizer : public UntaggedFinalizerBase { } template - friend void MournFinalized(GCVisitorType* visitor); + friend void MournFinalizerEntry(GCVisitorType*, FinalizerEntryPtr); friend class GCMarker; template friend class MarkingVisitorBase; @@ -3597,7 +3597,7 @@ class UntaggedFinalizerEntry : public UntaggedInstance { template friend class GCLinkedList; template - friend void MournFinalized(GCVisitorType* visitor); + friend void MournFinalizerEntry(GCVisitorType*, FinalizerEntryPtr); friend class GCMarker; template friend class MarkingVisitorBase;