diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 30e55ca974..0c33fcee03 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -4270,7 +4270,7 @@ void Heap::RegisterReservationsForBlackAllocation(Reservation* reservations) { void Heap::NotifyObjectLayoutChange(HeapObject* object, const DisallowHeapAllocation&) { if (FLAG_incremental_marking && incremental_marking()->IsMarking()) { - incremental_marking()->WhiteToGreyAndPush(object); + incremental_marking()->MarkGrey(object); } #ifdef VERIFY_HEAP DCHECK(pending_layout_change_object_ == nullptr); @@ -4834,7 +4834,7 @@ class IterateAndScavengePromotedObjectsVisitor final : public ObjectVisitor { // promoted objects. if (heap_->incremental_marking()->black_allocation()) { Code* code = Code::cast(Code::GetObjectFromEntryAddress(code_entry_slot)); - heap_->incremental_marking()->WhiteToGreyAndPush(code); + heap_->incremental_marking()->MarkGrey(code); } } @@ -5628,7 +5628,7 @@ void Heap::RegisterExternallyReferencedObject(Object** object) { HeapObject* heap_object = HeapObject::cast(*object); DCHECK(Contains(heap_object)); if (FLAG_incremental_marking_wrappers && incremental_marking()->IsMarking()) { - incremental_marking()->WhiteToGreyAndPush(heap_object); + incremental_marking()->MarkGrey(heap_object); } else { DCHECK(mark_compact_collector()->in_use()); mark_compact_collector()->MarkObject(heap_object); diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc index 5947e587db..a9a71cfb63 100644 --- a/src/heap/incremental-marking.cc +++ b/src/heap/incremental-marking.cc @@ -57,7 +57,9 @@ bool IncrementalMarking::BaseRecordWrite(HeapObject* obj, Object* value) { DCHECK(!ObjectMarking::IsImpossible(obj, marking_state(obj))); const bool is_black = ObjectMarking::IsBlack(obj, marking_state(obj)); - if (is_black && WhiteToGreyAndPush(value_heap_obj)) { + if (is_black && + ObjectMarking::IsWhite(value_heap_obj, marking_state(value_heap_obj))) { + WhiteToGreyAndPush(value_heap_obj); RestartIfNotMarking(); } return is_compacting_ && is_black; @@ -128,12 +130,9 @@ void IncrementalMarking::RecordWriteIntoCodeSlow(Code* host, RelocInfo* rinfo, } } -bool IncrementalMarking::WhiteToGreyAndPush(HeapObject* obj) { - if (ObjectMarking::WhiteToGrey(obj, marking_state(obj))) { - marking_deque()->Push(obj); - return true; - } - return false; +void IncrementalMarking::WhiteToGreyAndPush(HeapObject* obj) { + ObjectMarking::WhiteToGrey(obj, marking_state(obj)); + marking_deque()->Push(obj); } void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from, @@ -154,29 +153,9 @@ void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from, MarkBit old_mark_bit = ObjectMarking::MarkBitFrom(from, marking_state(from)); if (Marking::IsBlack(old_mark_bit)) { - if (from->address() + kPointerSize == to->address()) { - // The old and the new markbits overlap. The |to| object has the - // grey color. To make it black, we need to set second bit. - DCHECK(new_mark_bit.Get()); - new_mark_bit.Next().Set(); - } else { - bool success = Marking::WhiteToBlack(new_mark_bit); - DCHECK(success); - USE(success); - } + Marking::MarkBlack(new_mark_bit); } else if (Marking::IsGrey(old_mark_bit)) { - if (from->address() + kPointerSize == to->address()) { - // The old and the new markbits overlap. The |to| object has the - // white color. To make it black, we need to set both bits. - // Note that Marking::WhiteToGrey does not work here because - // old_mark_bit.Next() can be set by the concurrent marker at any time. - new_mark_bit.Set(); - new_mark_bit.Next().Set(); - } else { - bool success = Marking::WhiteToGrey(new_mark_bit); - DCHECK(success); - USE(success); - } + Marking::WhiteToGrey(new_mark_bit); marking_deque()->Push(to); RestartIfNotMarking(); } @@ -248,9 +227,12 @@ class IncrementalMarkingMarkingVisitor // Mark the object grey if it is white, do not enque it into the marking // deque. Heap* heap = map->GetHeap(); - bool ignored = ObjectMarking::WhiteToGrey( - heap_obj, heap->incremental_marking()->marking_state(heap_obj)); - USE(ignored); + if (ObjectMarking::IsWhite( + heap_obj, + heap->incremental_marking()->marking_state(heap_obj))) { + ObjectMarking::WhiteToGrey( + heap_obj, heap->incremental_marking()->marking_state(heap_obj)); + } } } VisitNativeContext(map, context); @@ -277,15 +259,21 @@ class IncrementalMarkingMarkingVisitor // Marks the object grey and pushes it on the marking stack. INLINE(static void MarkObject(Heap* heap, Object* obj)) { - heap->incremental_marking()->WhiteToGreyAndPush(HeapObject::cast(obj)); + heap->incremental_marking()->MarkGrey(HeapObject::cast(obj)); } // Marks the object black without pushing it on the marking stack. // Returns true if object needed marking and false otherwise. INLINE(static bool MarkObjectWithoutPush(Heap* heap, Object* obj)) { HeapObject* heap_object = HeapObject::cast(obj); - return ObjectMarking::WhiteToBlack( - heap_object, heap->incremental_marking()->marking_state(heap_object)); + if (ObjectMarking::IsWhite( + heap_object, + heap->incremental_marking()->marking_state(heap_object))) { + ObjectMarking::WhiteToBlack( + heap_object, heap->incremental_marking()->marking_state(heap_object)); + return true; + } + return false; } }; @@ -297,7 +285,7 @@ void IncrementalMarking::IterateBlackObject(HeapObject* object) { page->ResetProgressBar(); } Map* map = object->map(); - WhiteToGreyAndPush(map); + MarkGrey(map); IncrementalMarkingMarkingVisitor::IterateBody(map, object); } } @@ -321,7 +309,7 @@ class IncrementalMarkingRootMarkingVisitor : public RootVisitor { Object* obj = *p; if (!obj->IsHeapObject()) return; - heap_->incremental_marking()->WhiteToGreyAndPush(HeapObject::cast(obj)); + heap_->incremental_marking()->MarkGrey(HeapObject::cast(obj)); } Heap* heap_; @@ -712,7 +700,7 @@ void IncrementalMarking::RetainMaps() { if (i >= number_of_disposed_maps && !map_retaining_is_disabled && ObjectMarking::IsWhite(map, marking_state(map))) { if (ShouldRetainMap(map, age)) { - WhiteToGreyAndPush(map); + MarkGrey(map); } Object* prototype = map->prototype(); if (age > 0 && prototype->IsHeapObject() && @@ -832,7 +820,7 @@ void IncrementalMarking::UpdateMarkingDequeAfterScavenge() { void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) { - WhiteToGreyAndPush(map); + MarkGrey(map); IncrementalMarkingMarkingVisitor::IterateBody(map, obj); @@ -843,6 +831,17 @@ void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) { (chunk->IsFlagSet(MemoryChunk::HAS_PROGRESS_BAR) && Marking::IsBlack(mark_bit))); #endif + MarkBlack(obj, size); +} + +void IncrementalMarking::MarkGrey(HeapObject* object) { + if (ObjectMarking::IsWhite(object, marking_state(object))) { + WhiteToGreyAndPush(object); + } +} + +void IncrementalMarking::MarkBlack(HeapObject* obj, int size) { + if (ObjectMarking::IsBlack(obj, marking_state(obj))) return; ObjectMarking::GreyToBlack(obj, marking_state(obj)); } @@ -911,9 +910,9 @@ void IncrementalMarking::Hurry() { HeapObject* cache = HeapObject::cast( Context::cast(context)->get(Context::NORMALIZED_MAP_CACHE_INDEX)); if (!cache->IsUndefined(heap_->isolate())) { - // Mark the cache black if it is grey. - bool ignored = ObjectMarking::GreyToBlack(cache, marking_state(cache)); - USE(ignored); + if (ObjectMarking::IsGrey(cache, marking_state(cache))) { + ObjectMarking::GreyToBlack(cache, marking_state(cache)); + } } context = Context::cast(context)->next_context_link(); } diff --git a/src/heap/incremental-marking.h b/src/heap/incremental-marking.h index b412f0fffe..35978f3f8e 100644 --- a/src/heap/incremental-marking.h +++ b/src/heap/incremental-marking.h @@ -65,6 +65,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking { return MarkingState::Internal(chunk); } + void MarkBlack(HeapObject* object, int size); + void MarkGrey(HeapObject* object); + // Transfers mark bits without requiring proper object headers. void TransferMark(Heap* heap, HeapObject* from, HeapObject* to); @@ -79,15 +82,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking { DCHECK(ObjectMarking::IsWhite(to, marking_state(to))); if (ObjectMarking::IsGrey(from, marking_state(from))) { - bool success = - ObjectMarking::WhiteToGrey(to, marking_state(to)); - DCHECK(success); - USE(success); + ObjectMarking::WhiteToGrey(to, marking_state(to)); } else if (ObjectMarking::IsBlack(from, marking_state(from))) { - bool success = - ObjectMarking::WhiteToBlack(to, marking_state(to)); - DCHECK(success); - USE(success); + ObjectMarking::WhiteToBlack(to, marking_state(to)); } } @@ -213,9 +210,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking { void RecordCodeTargetPatch(Code* host, Address pc, HeapObject* value); void RecordCodeTargetPatch(Address pc, HeapObject* value); - // Returns true if the function succeeds in transitioning the object - // from white to grey. - bool WhiteToGreyAndPush(HeapObject* obj); + void WhiteToGreyAndPush(HeapObject* obj); inline void SetOldSpacePageFlags(MemoryChunk* chunk) { SetOldSpacePageFlags(chunk, IsMarking(), IsCompacting()); diff --git a/src/heap/mark-compact-inl.h b/src/heap/mark-compact-inl.h index 584f86dc88..cb280e2bbb 100644 --- a/src/heap/mark-compact-inl.h +++ b/src/heap/mark-compact-inl.h @@ -38,15 +38,19 @@ void MarkCompactCollector::UnshiftBlack(HeapObject* obj) { } void MarkCompactCollector::MarkObject(HeapObject* obj) { - if (ObjectMarking::WhiteToBlack( + if (ObjectMarking::IsWhite( obj, MarkingState::Internal(obj))) { + ObjectMarking::WhiteToBlack( + obj, MarkingState::Internal(obj)); PushBlack(obj); } } void MinorMarkCompactCollector::MarkObject(HeapObject* obj) { - if (ObjectMarking::WhiteToBlack( + if (ObjectMarking::IsWhite( obj, MarkingState::External(obj))) { + ObjectMarking::WhiteToBlack( + obj, MarkingState::External(obj)); PushBlack(obj); } } diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index cd9f548f53..8e1be75877 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -1196,10 +1196,12 @@ class StaticYoungGenerationMarkingVisitor StackLimitCheck check(heap->isolate()); if (check.HasOverflowed()) return false; - if (ObjectMarking::WhiteToBlack( - object, MarkingState::External(object))) { - IterateBody(object->map(), object); - } + if (ObjectMarking::IsBlackOrGrey( + object, MarkingState::External(object))) + return true; + ObjectMarking::WhiteToBlack( + object, MarkingState::External(object)); + IterateBody(object->map(), object); return true; } }; @@ -1235,7 +1237,11 @@ class MarkCompactMarkingVisitor // Marks the object black without pushing it on the marking stack. // Returns true if object needed marking and false otherwise. INLINE(static bool MarkObjectWithoutPush(Heap* heap, HeapObject* object)) { - return ObjectMarking::WhiteToBlack(object, MarkingState::Internal(object)); + if (ObjectMarking::IsWhite(object, MarkingState::Internal(object))) { + ObjectMarking::WhiteToBlack(object, MarkingState::Internal(object)); + return true; + } + return false; } // Mark object pointed to by p. @@ -1253,15 +1259,14 @@ class MarkCompactMarkingVisitor HeapObject* obj)) { #ifdef DEBUG DCHECK(collector->heap()->Contains(obj)); + DCHECK(ObjectMarking::IsWhite(obj, MarkingState::Internal(obj))); #endif - if (ObjectMarking::WhiteToBlack(obj, MarkingState::Internal(obj))) { - Map* map = obj->map(); - Heap* heap = obj->GetHeap(); - ObjectMarking::WhiteToBlack(obj, MarkingState::Internal(obj)); - // Mark the map pointer and the body. - heap->mark_compact_collector()->MarkObject(map); - IterateBody(map, obj); - } + Map* map = obj->map(); + Heap* heap = obj->GetHeap(); + ObjectMarking::WhiteToBlack(obj, MarkingState::Internal(obj)); + // Mark the map pointer and the body. + heap->mark_compact_collector()->MarkObject(map); + IterateBody(map, obj); } // Visit all unmarked objects pointed to by [start, end). @@ -1279,6 +1284,8 @@ class MarkCompactMarkingVisitor if (!o->IsHeapObject()) continue; collector->RecordSlot(object, p, o); HeapObject* obj = HeapObject::cast(o); + if (ObjectMarking::IsBlackOrGrey(obj, MarkingState::Internal(obj))) + continue; VisitUnmarkedObject(collector, obj); } return true; @@ -1475,12 +1482,16 @@ class MinorMarkCompactCollector::RootMarkingVisitor : public RootVisitor { if (!collector_->heap()->InNewSpace(object)) return; - if (ObjectMarking::WhiteToBlack( - object, MarkingState::External(object))) { - Map* map = object->map(); - StaticYoungGenerationMarkingVisitor::IterateBody(map, object); - collector_->EmptyMarkingDeque(); - } + if (ObjectMarking::IsBlackOrGrey( + object, MarkingState::External(object))) + return; + + Map* map = object->map(); + ObjectMarking::WhiteToBlack( + object, MarkingState::External(object)); + StaticYoungGenerationMarkingVisitor::IterateBody(map, object); + + collector_->EmptyMarkingDeque(); } MinorMarkCompactCollector* collector_; @@ -1521,16 +1532,22 @@ class MarkCompactCollector::RootMarkingVisitor : public ObjectVisitor, HeapObject* object = HeapObject::cast(*p); - if (ObjectMarking::WhiteToBlack( - object, MarkingState::Internal(object))) { - Map* map = object->map(); - // Mark the map pointer and body, and push them on the marking stack. - collector_->MarkObject(map); - MarkCompactMarkingVisitor::IterateBody(map, object); - // Mark all the objects reachable from the map and body. May leave - // overflowed objects in the heap. - collector_->EmptyMarkingDeque(); - } + if (ObjectMarking::IsBlackOrGrey( + object, MarkingState::Internal(object))) + return; + + Map* map = object->map(); + // Mark the object. + ObjectMarking::WhiteToBlack( + object, MarkingState::Internal(object)); + + // Mark the map pointer and body, and push them on the marking stack. + collector_->MarkObject(map); + MarkCompactMarkingVisitor::IterateBody(map, object); + + // Mark all the objects reachable from the map and body. May leave + // overflowed objects in the heap. + collector_->EmptyMarkingDeque(); } MarkCompactCollector* collector_; @@ -1701,7 +1718,8 @@ void MarkCompactCollector::DiscoverGreyObjectsWithIterator(T* it) { Map* filler_map = heap()->one_pointer_filler_map(); for (HeapObject* object = it->Next(); object != NULL; object = it->Next()) { if ((object->map() != filler_map) && - ObjectMarking::GreyToBlack(object, MarkingState::Internal(object))) { + ObjectMarking::IsGrey(object, MarkingState::Internal(object))) { + ObjectMarking::GreyToBlack(object, MarkingState::Internal(object)); PushBlack(object); if (marking_deque()->IsFull()) return; } @@ -1713,10 +1731,8 @@ void MarkCompactCollector::DiscoverGreyObjectsOnPage(MemoryChunk* p) { LiveObjectIterator it(p, MarkingState::Internal(p)); HeapObject* object = NULL; while ((object = it.Next()) != NULL) { - bool success = - ObjectMarking::GreyToBlack(object, MarkingState::Internal(object)); - DCHECK(success); - USE(success); + DCHECK(ObjectMarking::IsGrey(object, MarkingState::Internal(object))); + ObjectMarking::GreyToBlack(object, MarkingState::Internal(object)); PushBlack(object); if (marking_deque()->IsFull()) return; } @@ -2279,12 +2295,15 @@ bool MarkCompactCollector::IsUnmarkedHeapObject(Object** p) { void MarkCompactCollector::MarkStringTable(RootMarkingVisitor* visitor) { StringTable* string_table = heap()->string_table(); // Mark the string table itself. - if (ObjectMarking::WhiteToBlack(string_table, - MarkingState::Internal(string_table))) { - // Explicitly mark the prefix. - string_table->IteratePrefix(visitor); - ProcessMarkingDeque(); + if (ObjectMarking::IsWhite(string_table, + MarkingState::Internal(string_table))) { + // String table could have already been marked by visiting the handles list. + ObjectMarking::WhiteToBlack(string_table, + MarkingState::Internal(string_table)); } + // Explicitly mark the prefix. + string_table->IteratePrefix(visitor); + ProcessMarkingDeque(); } void MarkCompactCollector::MarkRoots(RootMarkingVisitor* visitor) { diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h index b727124b9c..2f7801098f 100644 --- a/src/heap/mark-compact.h +++ b/src/heap/mark-compact.h @@ -82,6 +82,8 @@ class ObjectMarking : public AllStatic { template V8_INLINE static bool BlackToGrey(HeapObject* obj, const MarkingState& state) { + DCHECK( + (access_mode == MarkBit::ATOMIC || IsBlack(obj, state))); MarkBit markbit = MarkBitFrom(obj, state); if (!Marking::BlackToGrey(markbit)) return false; state.IncrementLiveBytes(-obj->Size()); @@ -91,19 +93,24 @@ class ObjectMarking : public AllStatic { template V8_INLINE static bool WhiteToGrey(HeapObject* obj, const MarkingState& state) { + DCHECK( + (access_mode == MarkBit::ATOMIC || IsWhite(obj, state))); return Marking::WhiteToGrey(MarkBitFrom(obj, state)); } template V8_INLINE static bool WhiteToBlack(HeapObject* obj, const MarkingState& state) { - return ObjectMarking::WhiteToGrey(obj, state) && - ObjectMarking::GreyToBlack(obj, state); + DCHECK( + (access_mode == MarkBit::ATOMIC || IsWhite(obj, state))); + if (!ObjectMarking::WhiteToGrey(obj, state)) return false; + return ObjectMarking::GreyToBlack(obj, state); } template V8_INLINE static bool GreyToBlack(HeapObject* obj, const MarkingState& state) { + DCHECK((access_mode == MarkBit::ATOMIC || IsGrey(obj, state))); MarkBit markbit = MarkBitFrom(obj, state); if (!Marking::GreyToBlack(markbit)) return false; state.IncrementLiveBytes(obj->Size()); diff --git a/src/heap/marking.h b/src/heap/marking.h index ab98a124bc..b20a4d86f1 100644 --- a/src/heap/marking.h +++ b/src/heap/marking.h @@ -38,16 +38,12 @@ class MarkBit { } } - // The function returns true if it succeeded to - // transition the bit from 0 to 1. template inline bool Set(); template inline bool Get(); - // The function returns true if it succeeded to - // transition the bit from 1 to 0. template inline bool Clear(); @@ -61,9 +57,8 @@ class MarkBit { template <> inline bool MarkBit::Set() { - base::Atomic32 old_value = *cell_; - *cell_ = old_value | mask_; - return (old_value & mask_) == 0; + *cell_ |= mask_; + return true; } template <> @@ -91,9 +86,8 @@ inline bool MarkBit::Get() { template <> inline bool MarkBit::Clear() { - base::Atomic32 old_value = *cell_; - *cell_ = old_value & ~mask_; - return (old_value & mask_) == mask_; + *cell_ &= ~mask_; + return true; } template <> @@ -418,17 +412,24 @@ class Marking : public AllStatic { template INLINE(static bool WhiteToGrey(MarkBit markbit)) { + DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit)); return markbit.Set(); } + // Warning: this method is not safe in general in concurrent scenarios. + // If you know that nobody else will change the bits on the given location + // then you may use it. template - INLINE(static bool WhiteToBlack(MarkBit markbit)) { - return markbit.Set() && markbit.Next().Set(); + INLINE(static void WhiteToBlack(MarkBit markbit)) { + DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit)); + markbit.Set(); + markbit.Next().Set(); } template INLINE(static bool GreyToBlack(MarkBit markbit)) { - return markbit.Get() && markbit.Next().Set(); + DCHECK(mode == MarkBit::ATOMIC || IsGrey(markbit)); + return markbit.Next().Set(); } enum ObjectColor {