[heap] Add atomics to mark bit operations.

BUG=chromium:694255

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

src/heap/incremental-marking.cc

@@ -141,7 +141,7 @@ void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from,
   if (Marking::IsBlack(old_mark_bit)) {
     Marking::MarkBlack(new_mark_bit);
   } else if (Marking::IsGrey(old_mark_bit)) {
-    Marking::MarkGrey(new_mark_bit);
+    Marking::WhiteToGrey(new_mark_bit);
     heap->mark_compact_collector()->marking_deque()->Push(to);
     heap->incremental_marking()->RestartIfNotMarking();
   }

src/heap/mark-compact-inl.h

@@ -14,9 +14,9 @@ namespace internal {
 
 template <MarkingMode mode>
 void MarkCompactCollector::PushBlack(HeapObject* obj) {
-  DCHECK(ObjectMarking::IsBlack<mode>(obj));
+  DCHECK((ObjectMarking::IsBlack<MarkBit::NON_ATOMIC, mode>(obj)));
   if (!marking_deque<mode>()->Push(obj)) {
-    ObjectMarking::BlackToGrey<mode>(obj);
+    ObjectMarking::BlackToGrey<MarkBit::NON_ATOMIC, mode>(obj);
   }
 }
 
@@ -30,8 +30,8 @@ void MarkCompactCollector::UnshiftBlack(HeapObject* obj) {
 
 template <MarkingMode mode>
 void MarkCompactCollector::MarkObject(HeapObject* obj) {
-  if (ObjectMarking::IsWhite<mode>(obj)) {
-    ObjectMarking::WhiteToBlack<mode>(obj);
+  if (ObjectMarking::IsWhite<MarkBit::NON_ATOMIC, mode>(obj)) {
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC, mode>(obj);
     PushBlack<mode>(obj);
   }
 }

src/heap/mark-compact.cc

@@ -1087,9 +1087,11 @@ class StaticYoungGenerationMarkingVisitor
     StackLimitCheck check(heap->isolate());
     if (check.HasOverflowed()) return false;
 
-    if (ObjectMarking::IsBlackOrGrey<MarkingMode::YOUNG_GENERATION>(object))
+    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC,
+                                     MarkingMode::YOUNG_GENERATION>(object))
       return true;
-    ObjectMarking::WhiteToBlack<MarkingMode::YOUNG_GENERATION>(object);
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC,
+                                MarkingMode::YOUNG_GENERATION>(object);
     IterateBody(object->map(), object);
     return true;
   }
@@ -1368,11 +1370,11 @@ class RootMarkingVisitor : public ObjectVisitor {
         !collector_->heap()->InNewSpace(object))
       return;
 
-    if (ObjectMarking::IsBlackOrGrey<mode>(object)) return;
+    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC, mode>(object)) return;
 
     Map* map = object->map();
     // Mark the object.
-    ObjectMarking::WhiteToBlack<mode>(object);
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC, mode>(object);
 
     switch (mode) {
       case MarkingMode::FULL: {
@@ -1988,7 +1990,7 @@ void MarkCompactCollector::EmptyMarkingDeque() {
     DCHECK(!object->IsFiller());
     DCHECK(object->IsHeapObject());
     DCHECK(heap()->Contains(object));
-    DCHECK(!ObjectMarking::IsWhite<mode>(object));
+    DCHECK(!(ObjectMarking::IsWhite<MarkBit::NON_ATOMIC, mode>(object)));
 
     Map* map = object->map();
     switch (mode) {
@@ -1997,7 +1999,7 @@ void MarkCompactCollector::EmptyMarkingDeque() {
         MarkCompactMarkingVisitor::IterateBody(map, object);
       } break;
       case MarkingMode::YOUNG_GENERATION: {
-        DCHECK(ObjectMarking::IsBlack<mode>(object));
+        DCHECK((ObjectMarking::IsBlack<MarkBit::NON_ATOMIC, mode>(object)));
         StaticYoungGenerationMarkingVisitor::IterateBody(map, object);
       } break;
     }
@@ -2255,11 +2257,13 @@ SlotCallbackResult MarkCompactCollector::CheckAndMarkObject(
     // has to be in ToSpace.
     DCHECK(heap->InToSpace(object));
     HeapObject* heap_object = reinterpret_cast<HeapObject*>(object);
-    if (ObjectMarking::IsBlackOrGrey<MarkingMode::YOUNG_GENERATION>(
+    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC,
+                                     MarkingMode::YOUNG_GENERATION>(
             heap_object)) {
       return KEEP_SLOT;
     }
-    ObjectMarking::WhiteToBlack<MarkingMode::YOUNG_GENERATION>(heap_object);
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC,
+                                MarkingMode::YOUNG_GENERATION>(heap_object);
     StaticYoungGenerationMarkingVisitor::IterateBody(heap_object->map(),
                                                      heap_object);
     return KEEP_SLOT;

src/heap/mark-compact.h

@@ -47,59 +47,69 @@ class ObjectMarking : public AllStatic {
     return Marking::Color(ObjectMarking::MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
   V8_INLINE static bool IsImpossible(HeapObject* obj) {
-    return Marking::IsImpossible(MarkBitFrom<mode>(obj));
+    return Marking::IsImpossible<access_mode>(MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
   V8_INLINE static bool IsBlack(HeapObject* obj) {
-    return Marking::IsBlack(MarkBitFrom<mode>(obj));
+    return Marking::IsBlack<access_mode>(MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
   V8_INLINE static bool IsWhite(HeapObject* obj) {
-    return Marking::IsWhite(MarkBitFrom<mode>(obj));
+    return Marking::IsWhite<access_mode>(MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
   V8_INLINE static bool IsGrey(HeapObject* obj) {
-    return Marking::IsGrey(MarkBitFrom<mode>(obj));
+    return Marking::IsGrey<access_mode>(MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
   V8_INLINE static bool IsBlackOrGrey(HeapObject* obj) {
-    return Marking::IsBlackOrGrey(MarkBitFrom<mode>(obj));
+    return Marking::IsBlackOrGrey<access_mode>(MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
-  V8_INLINE static void BlackToGrey(HeapObject* obj) {
-    DCHECK(IsBlack<mode>(obj));
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
+  V8_INLINE static bool BlackToGrey(HeapObject* obj) {
+    DCHECK((access_mode == MarkBit::ATOMIC || IsBlack<access_mode, mode>(obj)));
     MarkBit markbit = MarkBitFrom<mode>(obj);
-    Marking::BlackToGrey(markbit);
+    if (!Marking::BlackToGrey<access_mode>(markbit)) return false;
     MemoryChunk::IncrementLiveBytes<mode>(obj, -obj->Size());
+    return true;
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
-  V8_INLINE static void WhiteToGrey(HeapObject* obj) {
-    DCHECK(IsWhite<mode>(obj));
-    Marking::WhiteToGrey(MarkBitFrom<mode>(obj));
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
+  V8_INLINE static bool WhiteToGrey(HeapObject* obj) {
+    DCHECK((access_mode == MarkBit::ATOMIC || IsWhite<access_mode, mode>(obj)));
+    return Marking::WhiteToGrey<access_mode>(MarkBitFrom<mode>(obj));
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
-  V8_INLINE static void WhiteToBlack(HeapObject* obj) {
-    DCHECK(IsWhite<mode>(obj));
-    MarkBit markbit = MarkBitFrom<mode>(obj);
-    Marking::WhiteToBlack(markbit);
-    MemoryChunk::IncrementLiveBytes<mode>(obj, obj->Size());
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
+  V8_INLINE static bool WhiteToBlack(HeapObject* obj) {
+    DCHECK((access_mode == MarkBit::ATOMIC || IsWhite<access_mode, mode>(obj)));
+    if (!ObjectMarking::WhiteToGrey<access_mode, mode>(obj)) return false;
+    return ObjectMarking::GreyToBlack<access_mode, mode>(obj);
   }
 
-  template <MarkingMode mode = MarkingMode::FULL>
-  V8_INLINE static void GreyToBlack(HeapObject* obj) {
-    DCHECK(IsGrey<mode>(obj));
+  template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC,
+            MarkingMode mode = MarkingMode::FULL>
+  V8_INLINE static bool GreyToBlack(HeapObject* obj) {
+    DCHECK((access_mode == MarkBit::ATOMIC || IsGrey<access_mode, mode>(obj)));
     MarkBit markbit = MarkBitFrom<mode>(obj);
-    Marking::GreyToBlack(markbit);
+    if (!Marking::GreyToBlack<access_mode>(markbit)) return false;
     MemoryChunk::IncrementLiveBytes<mode>(obj, obj->Size());
+    return true;
   }
 
  private:

src/heap/marking.h

@@ -5,6 +5,7 @@
 #ifndef V8_MARKING_H
 #define V8_MARKING_H
 
+#include "src/base/atomic-utils.h"
 #include "src/utils.h"
 
 namespace v8 {
@@ -13,8 +14,13 @@ namespace internal {
 class MarkBit {
  public:
   typedef uint32_t CellType;
+  STATIC_ASSERT(sizeof(CellType) == sizeof(base::Atomic32));
 
-  inline MarkBit(CellType* cell, CellType mask) : cell_(cell), mask_(mask) {}
+  enum AccessMode { ATOMIC, NON_ATOMIC };
+
+  inline MarkBit(base::Atomic32* cell, CellType mask) : cell_(cell) {
+    mask_ = static_cast<base::Atomic32>(mask);
+  }
 
 #ifdef DEBUG
   bool operator==(const MarkBit& other) {
@@ -23,9 +29,6 @@ class MarkBit {
 #endif
 
  private:
-  inline CellType* cell() { return cell_; }
-  inline CellType mask() { return mask_; }
-
   inline MarkBit Next() {
     CellType new_mask = mask_ << 1;
     if (new_mask == 0) {
@@ -35,12 +38,51 @@ class MarkBit {
     }
   }
 
-  inline void Set() { *cell_ |= mask_; }
-  inline bool Get() { return (*cell_ & mask_) != 0; }
-  inline void Clear() { *cell_ &= ~mask_; }
+  template <AccessMode mode = NON_ATOMIC>
+  inline bool Set() {
+    if (mode == ATOMIC) {
+      base::Atomic32 old_value;
+      base::Atomic32 new_value;
+      do {
+        old_value = base::NoBarrier_Load(cell_);
+        if (old_value & mask_) return false;
+        new_value = old_value | mask_;
+      } while (base::Release_CompareAndSwap(cell_, old_value, new_value) !=
+               old_value);
+    } else {
+      *cell_ |= mask_;
+    }
+    return true;
+  }
 
-  CellType* cell_;
-  CellType mask_;
+  template <AccessMode mode = NON_ATOMIC>
+  inline bool Get() {
+    if (mode == ATOMIC) {
+      return (base::Acquire_Load(cell_) & mask_) != 0;
+    } else {
+      return (base::NoBarrier_Load(cell_) & mask_) != 0;
+    }
+  }
+
+  template <AccessMode mode = NON_ATOMIC>
+  inline bool Clear() {
+    if (mode == ATOMIC) {
+      base::Atomic32 old_value;
+      base::Atomic32 new_value;
+      do {
+        old_value = base::NoBarrier_Load(cell_);
+        if (!(old_value & mask_)) return false;
+        new_value = old_value & ~mask_;
+      } while (base::Release_CompareAndSwap(cell_, old_value, new_value) !=
+               old_value);
+    } else {
+      *cell_ &= ~mask_;
+    }
+    return true;
+  }
+
+  base::Atomic32* cell_;
+  base::Atomic32 mask_;
 
   friend class IncrementalMarking;
   friend class ConcurrentMarkingMarkbits;
@@ -100,7 +142,7 @@ class Bitmap {
   inline MarkBit MarkBitFromIndex(uint32_t index) {
     MarkBit::CellType mask = 1u << IndexInCell(index);
     MarkBit::CellType* cell = this->cells() + (index >> kBitsPerCellLog2);
-    return MarkBit(cell, mask);
+    return MarkBit(reinterpret_cast<base::Atomic32*>(cell), mask);
   }
 
   void Clear() {
@@ -270,66 +312,100 @@ class Bitmap {
 
 class Marking : public AllStatic {
  public:
+  // TODO(hpayer): The current mark bit operations use as default NON_ATOMIC
+  // mode for access. We should remove the default value or switch it with
+  // ATOMIC as soon we add concurrency.
+
   // Impossible markbits: 01
   static const char* kImpossibleBitPattern;
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool IsImpossible(MarkBit mark_bit)) {
-    return !mark_bit.Get() && mark_bit.Next().Get();
+    if (mode == MarkBit::NON_ATOMIC) {
+      return !mark_bit.Get<mode>() && mark_bit.Next().Get<mode>();
+    }
+    // If we are in concurrent mode we can only tell if an object has the
+    // impossible bit pattern if we read the first bit again after reading
+    // the first and the second bit. If the first bit is till zero and the
+    // second bit is one then the object has the impossible bit pattern.
+    bool is_impossible = !mark_bit.Get<mode>() && mark_bit.Next().Get<mode>();
+    if (is_impossible) {
+      return !mark_bit.Get<mode>();
+    }
+    return false;
   }
 
   // Black markbits: 11
   static const char* kBlackBitPattern;
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool IsBlack(MarkBit mark_bit)) {
-    return mark_bit.Get() && mark_bit.Next().Get();
+    return mark_bit.Get<mode>() && mark_bit.Next().Get<mode>();
   }
 
   // White markbits: 00 - this is required by the mark bit clearer.
   static const char* kWhiteBitPattern;
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool IsWhite(MarkBit mark_bit)) {
     DCHECK(!IsImpossible(mark_bit));
-    return !mark_bit.Get();
+    return !mark_bit.Get<mode>();
   }
 
   // Grey markbits: 10
   static const char* kGreyBitPattern;
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool IsGrey(MarkBit mark_bit)) {
-    return mark_bit.Get() && !mark_bit.Next().Get();
+    return mark_bit.Get<mode>() && !mark_bit.Next().Get<mode>();
   }
 
   // IsBlackOrGrey assumes that the first bit is set for black or grey
   // objects.
-  INLINE(static bool IsBlackOrGrey(MarkBit mark_bit)) { return mark_bit.Get(); }
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
+  INLINE(static bool IsBlackOrGrey(MarkBit mark_bit)) {
+    return mark_bit.Get<mode>();
+  }
 
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static void MarkWhite(MarkBit markbit)) {
-    markbit.Clear();
-    markbit.Next().Clear();
+    STATIC_ASSERT(mode == MarkBit::NON_ATOMIC);
+    markbit.Clear<mode>();
+    markbit.Next().Clear<mode>();
   }
 
-  INLINE(static void MarkGrey(MarkBit markbit)) { 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 <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static void MarkBlack(MarkBit markbit)) {
-    markbit.Set();
-    markbit.Next().Set();
+    markbit.Set<mode>();
+    markbit.Next().Set<mode>();
   }
 
-  INLINE(static void BlackToGrey(MarkBit markbit)) {
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
+  INLINE(static bool BlackToGrey(MarkBit markbit)) {
+    STATIC_ASSERT(mode == MarkBit::NON_ATOMIC);
     DCHECK(IsBlack(markbit));
-    markbit.Next().Clear();
+    return markbit.Next().Clear<mode>();
   }
 
-  INLINE(static void WhiteToGrey(MarkBit markbit)) {
-    DCHECK(IsWhite(markbit));
-    markbit.Set();
+  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(IsWhite(markbit));
-    markbit.Set();
-    markbit.Next().Set();
+    DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit));
+    markbit.Set<mode>();
+    markbit.Next().Set<mode>();
   }
 
-  INLINE(static void GreyToBlack(MarkBit markbit)) {
-    DCHECK(IsGrey(markbit));
-    markbit.Next().Set();
+  template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
+  INLINE(static bool GreyToBlack(MarkBit markbit)) {
+    DCHECK(mode == MarkBit::ATOMIC || IsGrey(markbit));
+    return markbit.Next().Set<mode>();
   }
 
   enum ObjectColor {

test/unittests/heap/marking-unittest.cc

@@ -22,7 +22,7 @@ TEST(Marking, TransitionWhiteBlackWhite) {
     MarkBit mark_bit = bitmap->MarkBitFromIndex(position[i]);
     CHECK(Marking::IsWhite(mark_bit));
     CHECK(!Marking::IsImpossible(mark_bit));
-    Marking::WhiteToBlack(mark_bit);
+    Marking::WhiteToBlack<MarkBit::NON_ATOMIC>(mark_bit);
     CHECK(Marking::IsBlack(mark_bit));
     CHECK(!Marking::IsImpossible(mark_bit));
     Marking::MarkWhite(mark_bit);
@@ -43,11 +43,11 @@ TEST(Marking, TransitionWhiteGreyBlackGrey) {
     CHECK(Marking::IsWhite(mark_bit));
     CHECK(!Marking::IsBlackOrGrey(mark_bit));
     CHECK(!Marking::IsImpossible(mark_bit));
-    Marking::WhiteToGrey(mark_bit);
+    Marking::WhiteToGrey<MarkBit::NON_ATOMIC>(mark_bit);
     CHECK(Marking::IsGrey(mark_bit));
     CHECK(Marking::IsBlackOrGrey(mark_bit));
     CHECK(!Marking::IsImpossible(mark_bit));
-    Marking::GreyToBlack(mark_bit);
+    Marking::GreyToBlack<MarkBit::NON_ATOMIC>(mark_bit);
     CHECK(Marking::IsBlack(mark_bit));
     CHECK(Marking::IsBlackOrGrey(mark_bit));
     CHECK(!Marking::IsImpossible(mark_bit));