diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 5a44b609aa..6c5cf35964 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -666,7 +666,7 @@ DEFINE_BOOL(age_code, true, DEFINE_BOOL(incremental_marking, true, "use incremental marking") DEFINE_BOOL(incremental_marking_wrappers, true, "use incremental marking for marking wrappers") -DEFINE_BOOL(concurrent_marking, V8_CONCURRENT_MARKING, "use concurrent marking") +DEFINE_BOOL(concurrent_marking, false, "use concurrent marking") DEFINE_BOOL(trace_concurrent_marking, false, "trace concurrent marking") DEFINE_INT(min_progress_during_incremental_marking_finalization, 32, "keep finalizing incremental marking as long as we discover at " diff --git a/src/heap/concurrent-marking.cc b/src/heap/concurrent-marking.cc index 2e91ae368f..618cb1139b 100644 --- a/src/heap/concurrent-marking.cc +++ b/src/heap/concurrent-marking.cc @@ -30,11 +30,6 @@ class ConcurrentMarkingVisitor final explicit ConcurrentMarkingVisitor(ConcurrentMarkingDeque* deque) : deque_(deque) {} - bool ShouldVisit(HeapObject* object) override { - return ObjectMarking::GreyToBlack( - object, marking_state(object)); - } - void VisitPointers(HeapObject* host, Object** start, Object** end) override { for (Object** p = start; p < end; p++) { if (!(*p)->IsHeapObject()) continue; @@ -73,7 +68,7 @@ class ConcurrentMarkingVisitor final // =========================================================================== int VisitCode(Map* map, Code* object) override { - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): push the object to the bail-out deque. return 0; } @@ -82,65 +77,58 @@ class ConcurrentMarkingVisitor final // =========================================================================== int VisitBytecodeArray(Map* map, BytecodeArray* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitJSFunction(Map* map, JSFunction* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitMap(Map* map, Map* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitNativeContext(Map* map, Context* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitSharedFunctionInfo(Map* map, SharedFunctionInfo* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitTransitionArray(Map* map, TransitionArray* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitWeakCell(Map* map, WeakCell* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } int VisitJSWeakCollection(Map* map, JSWeakCollection* object) override { - // TODO(ulan): implement iteration of strong fields. - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kBailout); + // TODO(ulan): implement iteration of strong fields and push the object to + // the bailout deque. return 0; } - void MarkObject(HeapObject* object) { - if (ObjectMarking::WhiteToGrey( - object, marking_state(object))) { - deque_->Push(object, MarkingThread::kConcurrent, TargetDeque::kShared); - } + void MarkObject(HeapObject* obj) { + deque_->Push(obj, MarkingThread::kConcurrent, TargetDeque::kShared); } private: - MarkingState marking_state(HeapObject* object) const { - return MarkingState::Internal(object); - } - ConcurrentMarkingDeque* deque_; }; @@ -187,7 +175,7 @@ void ConcurrentMarking::Run() { TimedScope scope(&time_ms); HeapObject* object; while ((object = deque_->Pop(MarkingThread::kConcurrent)) != nullptr) { - bytes_marked += visitor_->Visit(object); + bytes_marked += visitor_->IterateBody(object); } } if (FLAG_trace_concurrent_marking) { 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 d9d42988f5..a9a71cfb63 100644 --- a/src/heap/incremental-marking.cc +++ b/src/heap/incremental-marking.cc @@ -52,13 +52,14 @@ IncrementalMarking::IncrementalMarking(Heap* heap) bool IncrementalMarking::BaseRecordWrite(HeapObject* obj, Object* value) { HeapObject* value_heap_obj = HeapObject::cast(value); - DCHECK(!ObjectMarking::IsImpossible( - value_heap_obj, marking_state(value_heap_obj))); - DCHECK(!ObjectMarking::IsImpossible(obj, marking_state(obj))); - const bool is_black = - ObjectMarking::IsBlack(obj, marking_state(obj)); + DCHECK(!ObjectMarking::IsImpossible(value_heap_obj, + marking_state(value_heap_obj))); + 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; @@ -129,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,32 +152,12 @@ void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from, MarkBit new_mark_bit = ObjectMarking::MarkBitFrom(to, marking_state(to)); 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); - } - } 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_deque()->Push(to); - RestartIfNotMarking(); - } + if (Marking::IsBlack(old_mark_bit)) { + Marking::MarkBlack(new_mark_bit); + } else if (Marking::IsGrey(old_mark_bit)) { + Marking::WhiteToGrey(new_mark_bit); + marking_deque()->Push(to); + RestartIfNotMarking(); } } @@ -220,11 +198,11 @@ class IncrementalMarkingMarkingVisitor } while (scan_until_end && start_offset < object_size); chunk->set_progress_bar(start_offset); if (start_offset < object_size) { - if (ObjectMarking::IsGrey( + if (ObjectMarking::IsGrey( object, heap->incremental_marking()->marking_state(object))) { heap->incremental_marking()->marking_deque()->Unshift(object); } else { - DCHECK(ObjectMarking::IsBlack( + DCHECK(ObjectMarking::IsBlack( object, heap->incremental_marking()->marking_state(object))); heap->mark_compact_collector()->UnshiftBlack(object); } @@ -249,10 +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); @@ -279,28 +259,33 @@ 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; } }; void IncrementalMarking::IterateBlackObject(HeapObject* object) { - if (IsMarking() && - ObjectMarking::IsBlack(object, marking_state(object))) { + if (IsMarking() && ObjectMarking::IsBlack(object, marking_state(object))) { Page* page = Page::FromAddress(object->address()); if ((page->owner() != nullptr) && (page->owner()->identity() == LO_SPACE)) { // IterateBlackObject requires us to visit the whole object. page->ResetProgressBar(); } Map* map = object->map(); - WhiteToGreyAndPush(map); + MarkGrey(map); IncrementalMarkingMarkingVisitor::IterateBody(map, object); } } @@ -324,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_; @@ -650,7 +635,7 @@ void IncrementalMarking::ProcessWeakCells() { HeapObject* value = HeapObject::cast(weak_cell->value()); // Remove weak cells with live objects from the list, they do not need // clearing. - if (ObjectMarking::IsBlackOrGrey(value, marking_state(value))) { + if (ObjectMarking::IsBlackOrGrey(value, marking_state(value))) { // Record slot, if value is pointing to an evacuation candidate. Object** slot = HeapObject::RawField(weak_cell, WeakCell::kValueOffset); heap_->mark_compact_collector()->RecordSlot(weak_cell, slot, *slot); @@ -681,10 +666,9 @@ bool ShouldRetainMap(Map* map, int age) { Object* constructor = map->GetConstructor(); Heap* heap = map->GetHeap(); if (!constructor->IsHeapObject() || - ObjectMarking::IsWhite( - HeapObject::cast(constructor), - heap->incremental_marking()->marking_state( - HeapObject::cast(constructor)))) { + ObjectMarking::IsWhite(HeapObject::cast(constructor), + heap->incremental_marking()->marking_state( + HeapObject::cast(constructor)))) { // The constructor is dead, no new objects with this map can // be created. Do not retain this map. return false; @@ -714,15 +698,14 @@ void IncrementalMarking::RetainMaps() { int new_age; Map* map = Map::cast(cell->value()); if (i >= number_of_disposed_maps && !map_retaining_is_disabled && - ObjectMarking::IsWhite(map, marking_state(map))) { + ObjectMarking::IsWhite(map, marking_state(map))) { if (ShouldRetainMap(map, age)) { - WhiteToGreyAndPush(map); + MarkGrey(map); } Object* prototype = map->prototype(); if (age > 0 && prototype->IsHeapObject() && - ObjectMarking::IsWhite( - HeapObject::cast(prototype), - marking_state(HeapObject::cast(prototype)))) { + ObjectMarking::IsWhite(HeapObject::cast(prototype), + marking_state(HeapObject::cast(prototype)))) { // The prototype is not marked, age the map. new_age = age - 1; } else { @@ -813,21 +796,21 @@ void IncrementalMarking::UpdateMarkingDequeAfterScavenge() { return nullptr; } HeapObject* dest = map_word.ToForwardingAddress(); - if (ObjectMarking::IsBlack(dest, marking_state(dest))) { + if (ObjectMarking::IsBlack(dest, marking_state(dest))) { // The object is already processed by the marker. return nullptr; } - DCHECK(ObjectMarking::IsGrey(obj, marking_state(obj)) || - (obj->IsFiller() && - ObjectMarking::IsWhite(obj, marking_state(obj)))); + DCHECK( + ObjectMarking::IsGrey(obj, marking_state(obj)) || + (obj->IsFiller() && ObjectMarking::IsWhite(obj, marking_state(obj)))); return dest; } else { - DCHECK(ObjectMarking::IsGrey(obj, marking_state(obj)) || + DCHECK(ObjectMarking::IsGrey(obj, marking_state(obj)) || (obj->IsFiller() && - ObjectMarking::IsWhite(obj, marking_state(obj))) || + ObjectMarking::IsWhite(obj, marking_state(obj))) || (MemoryChunk::FromAddress(obj->address()) ->IsFlagSet(MemoryChunk::HAS_PROGRESS_BAR) && - ObjectMarking::IsBlack(obj, marking_state(obj)))); + ObjectMarking::IsBlack(obj, marking_state(obj)))); // Skip one word filler objects that appear on the // stack when we perform in place array shift. return (obj->map() == filler_map) ? nullptr : obj; @@ -835,29 +818,33 @@ void IncrementalMarking::UpdateMarkingDequeAfterScavenge() { }); } -bool IncrementalMarking::IsFixedArrayWithProgressBar(HeapObject* obj) { - if (!obj->IsFixedArray()) return false; - MemoryChunk* chunk = MemoryChunk::FromAddress(obj->address()); - return chunk->IsFlagSet(MemoryChunk::HAS_PROGRESS_BAR); -} void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) { + MarkGrey(map); + + IncrementalMarkingMarkingVisitor::IterateBody(map, obj); + #if ENABLE_SLOW_DCHECKS MarkBit mark_bit = ObjectMarking::MarkBitFrom(obj, marking_state(obj)); MemoryChunk* chunk = MemoryChunk::FromAddress(obj->address()); - SLOW_DCHECK(Marking::IsGrey(mark_bit) || + SLOW_DCHECK(Marking::IsGrey(mark_bit) || (chunk->IsFlagSet(MemoryChunk::HAS_PROGRESS_BAR) && - Marking::IsBlack(mark_bit))); + Marking::IsBlack(mark_bit))); #endif - if (ObjectMarking::GreyToBlack(obj, marking_state(obj))) { - WhiteToGreyAndPush(map); - IncrementalMarkingMarkingVisitor::IterateBody(map, obj); - } else if (IsFixedArrayWithProgressBar(obj)) { - DCHECK(ObjectMarking::IsBlack(obj, marking_state(obj))); - IncrementalMarkingMarkingVisitor::VisitFixedArrayIncremental(map, obj); + 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)); +} + intptr_t IncrementalMarking::ProcessMarkingDeque( intptr_t bytes_to_process, ForceCompletionAction completion) { intptr_t bytes_processed = 0; @@ -868,7 +855,7 @@ intptr_t IncrementalMarking::ProcessMarkingDeque( // Left trimming may result in white, grey, or black filler objects on the // marking deque. Ignore these objects. if (obj->IsFiller()) { - DCHECK(!ObjectMarking::IsImpossible(obj, marking_state(obj))); + DCHECK(!ObjectMarking::IsImpossible(obj, marking_state(obj))); continue; } @@ -923,10 +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 0eaae2c699..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)); } } @@ -182,12 +179,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking { static const intptr_t kActivationThreshold = 0; #endif -#ifdef V8_CONCURRENT_MARKING - static const MarkBit::AccessMode kAtomicity = MarkBit::AccessMode::ATOMIC; -#else - static const MarkBit::AccessMode kAtomicity = MarkBit::AccessMode::NON_ATOMIC; -#endif - void FinalizeSweeping(); size_t Step(size_t bytes_to_process, CompletionAction action, @@ -219,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()); @@ -312,7 +301,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking { intptr_t bytes_to_process, ForceCompletionAction completion = DO_NOT_FORCE_COMPLETION)); - INLINE(bool IsFixedArrayWithProgressBar(HeapObject* object)); INLINE(void VisitObject(Map* map, HeapObject* obj, int size)); void IncrementIdleMarkingDelayCounter(); 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 { diff --git a/src/heap/objects-visiting-inl.h b/src/heap/objects-visiting-inl.h index 11bf679ec4..df3bab76a1 100644 --- a/src/heap/objects-visiting-inl.h +++ b/src/heap/objects-visiting-inl.h @@ -637,7 +637,8 @@ void StaticMarkingVisitor::VisitJSFunctionWeakCode( } template -ResultType HeapVisitor::Visit(HeapObject* object) { +ResultType HeapVisitor::IterateBody( + HeapObject* object) { Map* map = object->map(); ConcreteVisitor* visitor = static_cast(this); switch (static_cast(map->visitor_id())) { @@ -668,28 +669,14 @@ ResultType HeapVisitor::Visit(HeapObject* object) { return ResultType(); } -template -void HeapVisitor::VisitMapPointer( - HeapObject* host, HeapObject** map) { - static_cast(this)->VisitPointer( - host, reinterpret_cast(map)); -} - -template -bool HeapVisitor::ShouldVisit(HeapObject* object) { - return true; -} - -#define VISIT(type) \ - template \ - ResultType HeapVisitor::Visit##type( \ - Map* map, type* object) { \ - ConcreteVisitor* visitor = static_cast(this); \ - if (!visitor->ShouldVisit(object)) return ResultType(); \ - int size = type::BodyDescriptor::SizeOf(map, object); \ - visitor->VisitMapPointer(object, object->map_slot()); \ - type::BodyDescriptor::IterateBody(object, size, visitor); \ - return static_cast(size); \ +#define VISIT(type) \ + template \ + ResultType HeapVisitor::Visit##type( \ + Map* map, type* object) { \ + int size = type::BodyDescriptor::SizeOf(map, object); \ + type::BodyDescriptor::IterateBody(object, size, \ + static_cast(this)); \ + return static_cast(size); \ } TYPED_VISITOR_ID_LIST(VISIT) #undef VISIT @@ -697,10 +684,7 @@ TYPED_VISITOR_ID_LIST(VISIT) template ResultType HeapVisitor::VisitShortcutCandidate( Map* map, ConsString* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); int size = ConsString::BodyDescriptor::SizeOf(map, object); - visitor->VisitMapPointer(object, object->map_slot()); ConsString::BodyDescriptor::IterateBody(object, size, static_cast(this)); return static_cast(size); @@ -709,10 +693,7 @@ ResultType HeapVisitor::VisitShortcutCandidate( template ResultType HeapVisitor::VisitNativeContext( Map* map, Context* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); int size = Context::BodyDescriptor::SizeOf(map, object); - visitor->VisitMapPointer(object, object->map_slot()); Context::BodyDescriptor::IterateBody(object, size, static_cast(this)); return static_cast(size); @@ -721,20 +702,14 @@ ResultType HeapVisitor::VisitNativeContext( template ResultType HeapVisitor::VisitDataObject( Map* map, HeapObject* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); int size = map->instance_size(); - visitor->VisitMapPointer(object, object->map_slot()); return static_cast(size); } template ResultType HeapVisitor::VisitJSObjectFast( Map* map, JSObject* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); int size = JSObject::FastBodyDescriptor::SizeOf(map, object); - visitor->VisitMapPointer(object, object->map_slot()); JSObject::FastBodyDescriptor::IterateBody( object, size, static_cast(this)); return static_cast(size); @@ -742,10 +717,7 @@ ResultType HeapVisitor::VisitJSObjectFast( template ResultType HeapVisitor::VisitJSApiObject( Map* map, JSObject* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); int size = JSObject::BodyDescriptor::SizeOf(map, object); - visitor->VisitMapPointer(object, object->map_slot()); JSObject::BodyDescriptor::IterateBody(object, size, static_cast(this)); return static_cast(size); @@ -753,10 +725,7 @@ ResultType HeapVisitor::VisitJSApiObject( template ResultType HeapVisitor::VisitStruct( Map* map, HeapObject* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); int size = map->instance_size(); - visitor->VisitMapPointer(object, object->map_slot()); StructBodyDescriptor::IterateBody(object, size, static_cast(this)); return static_cast(size); @@ -764,9 +733,6 @@ ResultType HeapVisitor::VisitStruct( template ResultType HeapVisitor::VisitFreeSpace( Map* map, FreeSpace* object) { - ConcreteVisitor* visitor = static_cast(this); - if (!visitor->ShouldVisit(object)) return ResultType(); - visitor->VisitMapPointer(object, object->map_slot()); return static_cast(FreeSpace::cast(object)->size()); } diff --git a/src/heap/objects-visiting.h b/src/heap/objects-visiting.h index c578a42d64..c9d1c05c93 100644 --- a/src/heap/objects-visiting.h +++ b/src/heap/objects-visiting.h @@ -395,16 +395,9 @@ VisitorDispatchTable::Callback> template class HeapVisitor : public ObjectVisitor { public: - ResultType Visit(HeapObject* object); + ResultType IterateBody(HeapObject* object); protected: - // A guard predicate for visiting the object. - // If it returns false then the default implementations of the Visit* - // functions bailout from iterating the object pointers. - virtual bool ShouldVisit(HeapObject* object); - // A callback for visiting the map pointer in the object header. - virtual void VisitMapPointer(HeapObject* host, HeapObject** map); - #define VISIT(type) virtual ResultType Visit##type(Map* map, type* object); TYPED_VISITOR_ID_LIST(VISIT) #undef VISIT diff --git a/src/objects-inl.h b/src/objects-inl.h index 9e91da2cdc..a3f87c4b24 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1525,9 +1525,6 @@ void HeapObject::set_map_no_write_barrier(Map* value) { set_map_word(MapWord::FromMap(value)); } -HeapObject** HeapObject::map_slot() { - return reinterpret_cast(FIELD_ADDR(this, kMapOffset)); -} MapWord HeapObject::map_word() const { return MapWord( diff --git a/src/objects.h b/src/objects.h index c73f28da5e..4c72ad5b2d 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1669,9 +1669,6 @@ class HeapObject: public Object { // information. inline Map* map() const; inline void set_map(Map* value); - - inline HeapObject** map_slot(); - // The no-write-barrier version. This is OK if the object is white and in // new space, or if the value is an immortal immutable object, like the maps // of primitive (non-JS) objects like strings, heap numbers etc.