[heap] Make non-atomic markbit operations consistent with atomic ones.

Now non-atomic color transition operations return a boolean indicating
whether the transition succeeded or not.

This allows to replace color check and transition operations with a
single transition operation. For example:

if (IsWhite(object)) {
  WhiteToBlack(object);
  Foo();
}

becomes

if (WhiteToBlack(object)) {
  Foo();
}

BUG=chromium:694255

Review-Url: https://codereview.chromium.org/2857713002
Cr-Commit-Position: refs/heads/master@{#45085}

src/heap/heap.cc

@@ -4267,7 +4267,7 @@ void Heap::RegisterReservationsForBlackAllocation(Reservation* reservations) {
 void Heap::NotifyObjectLayoutChange(HeapObject* object,
                                     const DisallowHeapAllocation&) {
   if (FLAG_incremental_marking && incremental_marking()->IsMarking()) {
-    incremental_marking()->MarkGrey(object);
+    incremental_marking()->WhiteToGreyAndPush(object);
   }
 #ifdef VERIFY_HEAP
   DCHECK(pending_layout_change_object_ == nullptr);
@@ -4831,7 +4831,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()->MarkGrey(code);
+      heap_->incremental_marking()->WhiteToGreyAndPush(code);
     }
   }
 
@@ -5625,7 +5625,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()->MarkGrey(heap_object);
+    incremental_marking()->WhiteToGreyAndPush(heap_object);
   } else {
     DCHECK(mark_compact_collector()->in_use());
     mark_compact_collector()->MarkObject(heap_object);

src/heap/incremental-marking.cc

@@ -57,9 +57,7 @@ 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 &&
-      ObjectMarking::IsWhite(value_heap_obj, marking_state(value_heap_obj))) {
-    WhiteToGreyAndPush(value_heap_obj);
+  if (is_black && WhiteToGreyAndPush(value_heap_obj)) {
     RestartIfNotMarking();
   }
   return is_compacting_ && is_black;
@@ -130,9 +128,12 @@ void IncrementalMarking::RecordWriteIntoCodeSlow(Code* host, RelocInfo* rinfo,
   }
 }
 
-void IncrementalMarking::WhiteToGreyAndPush(HeapObject* obj) {
-  ObjectMarking::WhiteToGrey(obj, marking_state(obj));
+bool IncrementalMarking::WhiteToGreyAndPush(HeapObject* obj) {
+  if (ObjectMarking::WhiteToGrey(obj, marking_state(obj))) {
     marking_deque()->Push(obj);
+    return true;
+  }
+  return false;
 }
 
 void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from,
@@ -153,12 +154,32 @@ void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from,
   MarkBit old_mark_bit = ObjectMarking::MarkBitFrom(from, marking_state(from));
 
   if (Marking::IsBlack(old_mark_bit)) {
-    Marking::MarkBlack(new_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)) {
-    Marking::WhiteToGrey(new_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();
     }
+  }
 }
 
 class IncrementalMarkingMarkingVisitor
@@ -227,12 +248,9 @@ class IncrementalMarkingMarkingVisitor
         // Mark the object grey if it is white, do not enque it into the marking
         // deque.
         Heap* heap = map->GetHeap();
-        if (ObjectMarking::IsWhite(
-                heap_obj,
-                heap->incremental_marking()->marking_state(heap_obj))) {
-          ObjectMarking::WhiteToGrey(
+        bool ignored = ObjectMarking::WhiteToGrey(
             heap_obj, heap->incremental_marking()->marking_state(heap_obj));
-        }
+        USE(ignored);
       }
     }
     VisitNativeContext(map, context);
@@ -259,21 +277,15 @@ class IncrementalMarkingMarkingVisitor
 
   // Marks the object grey and pushes it on the marking stack.
   INLINE(static void MarkObject(Heap* heap, Object* obj)) {
-    heap->incremental_marking()->MarkGrey(HeapObject::cast(obj));
+    heap->incremental_marking()->WhiteToGreyAndPush(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);
-    if (ObjectMarking::IsWhite(
-            heap_object,
-            heap->incremental_marking()->marking_state(heap_object))) {
-      ObjectMarking::WhiteToBlack(
+    return ObjectMarking::WhiteToBlack(
         heap_object, heap->incremental_marking()->marking_state(heap_object));
-      return true;
-    }
-    return false;
   }
 };
 
@@ -285,7 +297,7 @@ void IncrementalMarking::IterateBlackObject(HeapObject* object) {
       page->ResetProgressBar();
     }
     Map* map = object->map();
-    MarkGrey(map);
+    WhiteToGreyAndPush(map);
     IncrementalMarkingMarkingVisitor::IterateBody(map, object);
   }
 }
@@ -309,7 +321,7 @@ class IncrementalMarkingRootMarkingVisitor : public RootVisitor {
     Object* obj = *p;
     if (!obj->IsHeapObject()) return;
 
-    heap_->incremental_marking()->MarkGrey(HeapObject::cast(obj));
+    heap_->incremental_marking()->WhiteToGreyAndPush(HeapObject::cast(obj));
   }
 
   Heap* heap_;
@@ -700,7 +712,7 @@ void IncrementalMarking::RetainMaps() {
     if (i >= number_of_disposed_maps && !map_retaining_is_disabled &&
         ObjectMarking::IsWhite(map, marking_state(map))) {
       if (ShouldRetainMap(map, age)) {
-        MarkGrey(map);
+        WhiteToGreyAndPush(map);
       }
       Object* prototype = map->prototype();
       if (age > 0 && prototype->IsHeapObject() &&
@@ -820,7 +832,7 @@ void IncrementalMarking::UpdateMarkingDequeAfterScavenge() {
 
 
 void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) {
-  MarkGrey(map);
+  WhiteToGreyAndPush(map);
 
   IncrementalMarkingMarkingVisitor::IterateBody(map, obj);
 
@@ -831,17 +843,6 @@ 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));
 }
 
@@ -910,9 +911,9 @@ void IncrementalMarking::Hurry() {
     HeapObject* cache = HeapObject::cast(
         Context::cast(context)->get(Context::NORMALIZED_MAP_CACHE_INDEX));
     if (!cache->IsUndefined(heap_->isolate())) {
-      if (ObjectMarking::IsGrey(cache, marking_state(cache))) {
-        ObjectMarking::GreyToBlack(cache, marking_state(cache));
-      }
+      // Mark the cache black if it is grey.
+      bool ignored = ObjectMarking::GreyToBlack(cache, marking_state(cache));
+      USE(ignored);
     }
     context = Context::cast(context)->next_context_link();
   }

src/heap/incremental-marking.h

@@ -65,9 +65,6 @@ 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);
 
@@ -82,9 +79,15 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
 
     DCHECK(ObjectMarking::IsWhite<access_mode>(to, marking_state(to)));
     if (ObjectMarking::IsGrey<access_mode>(from, marking_state(from))) {
+      bool success =
           ObjectMarking::WhiteToGrey<access_mode>(to, marking_state(to));
+      DCHECK(success);
+      USE(success);
     } else if (ObjectMarking::IsBlack<access_mode>(from, marking_state(from))) {
+      bool success =
           ObjectMarking::WhiteToBlack<access_mode>(to, marking_state(to));
+      DCHECK(success);
+      USE(success);
     }
   }
 
@@ -210,7 +213,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
   void RecordCodeTargetPatch(Code* host, Address pc, HeapObject* value);
   void RecordCodeTargetPatch(Address pc, HeapObject* value);
 
-  void WhiteToGreyAndPush(HeapObject* obj);
+  // Returns true if the function succeeds in transitioning the object
+  // from white to grey.
+  bool WhiteToGreyAndPush(HeapObject* obj);
 
   inline void SetOldSpacePageFlags(MemoryChunk* chunk) {
     SetOldSpacePageFlags(chunk, IsMarking(), IsCompacting());

src/heap/mark-compact-inl.h

@@ -38,19 +38,15 @@ void MarkCompactCollector::UnshiftBlack(HeapObject* obj) {
 }
 
 void MarkCompactCollector::MarkObject(HeapObject* obj) {
-  if (ObjectMarking::IsWhite<MarkBit::NON_ATOMIC>(
+  if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
           obj, MarkingState::Internal(obj))) {
-    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-        obj, MarkingState::Internal(obj));
     PushBlack(obj);
   }
 }
 
 void MinorMarkCompactCollector::MarkObject(HeapObject* obj) {
-  if (ObjectMarking::IsWhite<MarkBit::NON_ATOMIC>(
+  if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
           obj, MarkingState::External(obj))) {
-    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-        obj, MarkingState::External(obj));
     PushBlack(obj);
   }
 }

src/heap/mark-compact.cc

@@ -1196,12 +1196,10 @@ class StaticYoungGenerationMarkingVisitor
     StackLimitCheck check(heap->isolate());
     if (check.HasOverflowed()) return false;
 
-    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC>(
-            object, MarkingState::External(object)))
-      return true;
-    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-        object, MarkingState::External(object));
+    if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+            object, MarkingState::External(object))) {
       IterateBody(object->map(), object);
+    }
     return true;
   }
 };
@@ -1237,11 +1235,7 @@ 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)) {
-    if (ObjectMarking::IsWhite(object, MarkingState::Internal(object))) {
-      ObjectMarking::WhiteToBlack(object, MarkingState::Internal(object));
-      return true;
-    }
-    return false;
+    return ObjectMarking::WhiteToBlack(object, MarkingState::Internal(object));
   }
 
   // Mark object pointed to by p.
@@ -1259,8 +1253,8 @@ 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));
@@ -1268,6 +1262,7 @@ class MarkCompactMarkingVisitor
       heap->mark_compact_collector()->MarkObject(map);
       IterateBody(map, obj);
     }
+  }
 
   // Visit all unmarked objects pointed to by [start, end).
   // Returns false if the operation fails (lack of stack space).
@@ -1284,8 +1279,6 @@ 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;
@@ -1482,17 +1475,13 @@ class MinorMarkCompactCollector::RootMarkingVisitor : public RootVisitor {
 
     if (!collector_->heap()->InNewSpace(object)) return;
 
-    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC>(
-            object, MarkingState::External(object)))
-      return;
-
+    if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+            object, MarkingState::External(object))) {
       Map* map = object->map();
-    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-        object, MarkingState::External(object));
       StaticYoungGenerationMarkingVisitor::IterateBody(map, object);
-
       collector_->EmptyMarkingDeque();
     }
+  }
 
   MinorMarkCompactCollector* collector_;
 };
@@ -1532,23 +1521,17 @@ class MarkCompactCollector::RootMarkingVisitor : public ObjectVisitor,
 
     HeapObject* object = HeapObject::cast(*p);
 
-    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC>(
-            object, MarkingState::Internal(object)))
-      return;
-
+    if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+            object, MarkingState::Internal(object))) {
       Map* map = object->map();
-    // Mark the object.
-    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-        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_;
 };
@@ -1718,8 +1701,7 @@ 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::IsGrey(object, MarkingState::Internal(object))) {
-      ObjectMarking::GreyToBlack(object, MarkingState::Internal(object));
+        ObjectMarking::GreyToBlack(object, MarkingState::Internal(object))) {
       PushBlack(object);
       if (marking_deque()->IsFull()) return;
     }
@@ -1731,8 +1713,10 @@ void MarkCompactCollector::DiscoverGreyObjectsOnPage(MemoryChunk* p) {
   LiveObjectIterator<kGreyObjects> it(p, MarkingState::Internal(p));
   HeapObject* object = NULL;
   while ((object = it.Next()) != NULL) {
-    DCHECK(ObjectMarking::IsGrey(object, MarkingState::Internal(object)));
+    bool success =
         ObjectMarking::GreyToBlack(object, MarkingState::Internal(object));
+    DCHECK(success);
+    USE(success);
     PushBlack(object);
     if (marking_deque()->IsFull()) return;
   }
@@ -2295,15 +2279,12 @@ bool MarkCompactCollector::IsUnmarkedHeapObject(Object** p) {
 void MarkCompactCollector::MarkStringTable(RootMarkingVisitor* visitor) {
   StringTable* string_table = heap()->string_table();
   // Mark the string table itself.
-  if (ObjectMarking::IsWhite(string_table,
+  if (ObjectMarking::WhiteToBlack(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) {

src/heap/mark-compact.h

@@ -82,8 +82,6 @@ class ObjectMarking : public AllStatic {
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool BlackToGrey(HeapObject* obj,
                                     const MarkingState& state) {
-    DCHECK(
-        (access_mode == MarkBit::ATOMIC || IsBlack<access_mode>(obj, state)));
     MarkBit markbit = MarkBitFrom(obj, state);
     if (!Marking::BlackToGrey<access_mode>(markbit)) return false;
     state.IncrementLiveBytes<access_mode>(-obj->Size());
@@ -93,24 +91,19 @@ class ObjectMarking : public AllStatic {
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool WhiteToGrey(HeapObject* obj,
                                     const MarkingState& state) {
-    DCHECK(
-        (access_mode == MarkBit::ATOMIC || IsWhite<access_mode>(obj, state)));
     return Marking::WhiteToGrey<access_mode>(MarkBitFrom(obj, state));
   }
 
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool WhiteToBlack(HeapObject* obj,
                                      const MarkingState& state) {
-    DCHECK(
-        (access_mode == MarkBit::ATOMIC || IsWhite<access_mode>(obj, state)));
-    if (!ObjectMarking::WhiteToGrey<access_mode>(obj, state)) return false;
-    return ObjectMarking::GreyToBlack<access_mode>(obj, state);
+    return ObjectMarking::WhiteToGrey<access_mode>(obj, state) &&
+           ObjectMarking::GreyToBlack<access_mode>(obj, state);
   }
 
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool GreyToBlack(HeapObject* obj,
                                     const MarkingState& state) {
-    DCHECK((access_mode == MarkBit::ATOMIC || IsGrey<access_mode>(obj, state)));
     MarkBit markbit = MarkBitFrom(obj, state);
     if (!Marking::GreyToBlack<access_mode>(markbit)) return false;
     state.IncrementLiveBytes<access_mode>(obj->Size());

src/heap/marking.h

@@ -38,12 +38,16 @@ class MarkBit {
     }
   }
 
+  // The function returns true if it succeeded to
+  // transition the bit from 0 to 1.
   template <AccessMode mode = NON_ATOMIC>
   inline bool Set();
 
   template <AccessMode mode = NON_ATOMIC>
   inline bool Get();
 
+  // The function returns true if it succeeded to
+  // transition the bit from 1 to 0.
   template <AccessMode mode = NON_ATOMIC>
   inline bool Clear();
 
@@ -57,8 +61,9 @@ class MarkBit {
 
 template <>
 inline bool MarkBit::Set<MarkBit::NON_ATOMIC>() {
-  *cell_ |= mask_;
-  return true;
+  base::Atomic32 old_value = *cell_;
+  *cell_ = old_value | mask_;
+  return (old_value & mask_) == 0;
 }
 
 template <>
@@ -86,8 +91,9 @@ inline bool MarkBit::Get<MarkBit::ATOMIC>() {
 
 template <>
 inline bool MarkBit::Clear<MarkBit::NON_ATOMIC>() {
-  *cell_ &= ~mask_;
-  return true;
+  base::Atomic32 old_value = *cell_;
+  *cell_ = old_value & ~mask_;
+  return (old_value & mask_) == mask_;
 }
 
 template <>
@@ -412,24 +418,17 @@ class Marking : public AllStatic {
 
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool WhiteToGrey(MarkBit markbit)) {
-    DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit));
     return markbit.Set<mode>();
   }
 
-  // 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 <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
-  INLINE(static void WhiteToBlack(MarkBit markbit)) {
-    DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit));
-    markbit.Set<mode>();
-    markbit.Next().Set<mode>();
+  INLINE(static bool WhiteToBlack(MarkBit markbit)) {
+    return markbit.Set<mode>() && markbit.Next().Set<mode>();
   }
 
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool GreyToBlack(MarkBit markbit)) {
-    DCHECK(mode == MarkBit::ATOMIC || IsGrey(markbit));
-    return markbit.Next().Set<mode>();
+    return markbit.Get<mode>() && markbit.Next().Set<mode>();
   }
 
   enum ObjectColor {