Immediately promote marked objects during scavenge

It happens that a scavenger runs during incremental marking. Currently scavenger does not care about MarkCompact's mark bits. When an object is alive and marked, and at least one scavenge happens during incremental marking, the object will be copied once to the other semispace in the new_space, and then once to the old_space. For surviving objects this is useless extra work.

In our current attempts (https://codereview.chromium.org/1988623002) to ensure marked objects are scavenged, all marked objects will survive therefore there will be many objects which will be uselessly copied.

This cl modifies our promotion logic so when incremental marking is in progress, and the object is marked, we promote it unconditionally.

BUG=
LOG=no

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

src/heap/heap-inl.h

@@ -393,14 +393,30 @@ bool Heap::OldGenerationAllocationLimitReached() {
   return OldGenerationSpaceAvailable() < 0;
 }
 
-
+template <PromotionMode promotion_mode>
 bool Heap::ShouldBePromoted(Address old_address, int object_size) {
   Page* page = Page::FromAddress(old_address);
   Address age_mark = new_space_.age_mark();
+
+  if (promotion_mode == PROMOTE_MARKED) {
+    MarkBit mark_bit = Marking::MarkBitFrom(old_address);
+    if (!Marking::IsWhite(mark_bit)) {
+      return true;
+    }
+  }
+
   return page->IsFlagSet(MemoryChunk::NEW_SPACE_BELOW_AGE_MARK) &&
          (!page->ContainsLimit(age_mark) || old_address < age_mark);
 }
 
+PromotionMode Heap::CurrentPromotionMode() {
+  if (incremental_marking()->IsMarking()) {
+    return PROMOTE_MARKED;
+  } else {
+    return DEFAULT_PROMOTION;
+  }
+}
+
 void Heap::RecordWrite(Object* object, int offset, Object* o) {
   if (!InNewSpace(o) || !object->IsHeapObject() || InNewSpace(object)) {
     return;

src/heap/heap.cc

@@ -1653,6 +1653,7 @@ void Heap::Scavenge() {
   Address new_space_front = new_space_.ToSpaceStart();
   promotion_queue_.Initialize();
 
+  PromotionMode promotion_mode = CurrentPromotionMode();
   ScavengeVisitor scavenge_visitor(this);
 
   if (FLAG_scavenge_reclaim_unmodified_objects) {
@@ -1670,7 +1671,7 @@ void Heap::Scavenge() {
     // Copy objects reachable from the old generation.
     TRACE_GC(tracer(), GCTracer::Scope::SCAVENGER_OLD_TO_NEW_POINTERS);
     RememberedSet<OLD_TO_NEW>::Iterate(this, [this](Address addr) {
-      return Scavenger::CheckAndScavengeObject(this, addr, DEFAULT_PROMOTION);
+      return Scavenger::CheckAndScavengeObject(this, addr);
     });
 
     RememberedSet<OLD_TO_NEW>::IterateTyped(
@@ -1681,7 +1682,7 @@ void Heap::Scavenge() {
                 // If we do not force promotion, then we need to clear
                 // old_to_new slots in dead code objects after mark-compact.
                 return Scavenger::CheckAndScavengeObject(
-                    this, reinterpret_cast<Address>(addr), FORCE_PROMOTION);
+                    this, reinterpret_cast<Address>(addr));
               });
         });
   }
@@ -1705,7 +1706,8 @@ void Heap::Scavenge() {
 
   {
     TRACE_GC(tracer(), GCTracer::Scope::SCAVENGER_SEMISPACE);
-    new_space_front = DoScavenge(&scavenge_visitor, new_space_front);
+    new_space_front =
+        DoScavenge(&scavenge_visitor, new_space_front, promotion_mode);
   }
 
   if (FLAG_scavenge_reclaim_unmodified_objects) {
@@ -1714,12 +1716,14 @@ void Heap::Scavenge() {
 
     isolate()->global_handles()->IterateNewSpaceWeakUnmodifiedRoots(
         &scavenge_visitor);
-    new_space_front = DoScavenge(&scavenge_visitor, new_space_front);
+    new_space_front =
+        DoScavenge(&scavenge_visitor, new_space_front, promotion_mode);
   } else {
     TRACE_GC(tracer(), GCTracer::Scope::SCAVENGER_OBJECT_GROUPS);
     while (isolate()->global_handles()->IterateObjectGroups(
         &scavenge_visitor, &IsUnscavengedHeapObject)) {
-      new_space_front = DoScavenge(&scavenge_visitor, new_space_front);
+      new_space_front =
+          DoScavenge(&scavenge_visitor, new_space_front, promotion_mode);
     }
     isolate()->global_handles()->RemoveObjectGroups();
     isolate()->global_handles()->RemoveImplicitRefGroups();
@@ -1729,7 +1733,8 @@ void Heap::Scavenge() {
 
     isolate()->global_handles()->IterateNewSpaceWeakIndependentRoots(
         &scavenge_visitor);
-    new_space_front = DoScavenge(&scavenge_visitor, new_space_front);
+    new_space_front =
+        DoScavenge(&scavenge_visitor, new_space_front, promotion_mode);
   }
 
   UpdateNewSpaceReferencesInExternalStringTable(
@@ -1911,9 +1916,9 @@ void Heap::VisitExternalResources(v8::ExternalResourceVisitor* visitor) {
   external_string_table_.Iterate(&external_string_table_visitor);
 }
 
-
 Address Heap::DoScavenge(ObjectVisitor* scavenge_visitor,
-                         Address new_space_front) {
+                         Address new_space_front,
+                         PromotionMode promotion_mode) {
   do {
     SemiSpace::AssertValidRange(new_space_front, new_space_.top());
     // The addresses new_space_front and new_space_.top() define a
@@ -1922,8 +1927,14 @@ Address Heap::DoScavenge(ObjectVisitor* scavenge_visitor,
     while (new_space_front != new_space_.top()) {
       if (!Page::IsAlignedToPageSize(new_space_front)) {
         HeapObject* object = HeapObject::FromAddress(new_space_front);
+        if (promotion_mode == PROMOTE_MARKED) {
+          new_space_front += StaticScavengeVisitor<PROMOTE_MARKED>::IterateBody(
+              object->map(), object);
+        } else {
           new_space_front +=
-            StaticScavengeVisitor::IterateBody(object->map(), object);
+              StaticScavengeVisitor<DEFAULT_PROMOTION>::IterateBody(
+                  object->map(), object);
+        }
       } else {
         new_space_front = Page::FromAllocationAreaAddress(new_space_front)
                               ->next_page()
@@ -4689,8 +4700,8 @@ void Heap::IteratePromotedObjectPointers(HeapObject* object, Address start,
     Object* target = *slot;
     if (target->IsHeapObject()) {
       if (Heap::InFromSpace(target)) {
-        callback(reinterpret_cast<HeapObject**>(slot), HeapObject::cast(target),
-                 DEFAULT_PROMOTION);
+        callback(reinterpret_cast<HeapObject**>(slot),
+                 HeapObject::cast(target));
         Object* new_target = *slot;
         if (InNewSpace(new_target)) {
           SLOW_DCHECK(Heap::InToSpace(new_target));
@@ -5232,7 +5243,8 @@ V8_DECLARE_ONCE(initialize_gc_once);
 
 static void InitializeGCOnce() {
   Scavenger::Initialize();
-  StaticScavengeVisitor::Initialize();
+  StaticScavengeVisitor<DEFAULT_PROMOTION>::Initialize();
+  StaticScavengeVisitor<PROMOTE_MARKED>::Initialize();
   MarkCompactCollector::Initialize();
 }
 

src/heap/heap.h

@@ -303,10 +303,9 @@ class Scavenger;
 class ScavengeJob;
 class WeakObjectRetainer;
 
-enum PromotionMode { FORCE_PROMOTION, DEFAULT_PROMOTION };
+enum PromotionMode { PROMOTE_MARKED, DEFAULT_PROMOTION };
 
-typedef void (*ObjectSlotCallback)(HeapObject** from, HeapObject* to,
-                                   PromotionMode mode);
+typedef void (*ObjectSlotCallback)(HeapObject** from, HeapObject* to);
 
 // A queue of objects promoted during scavenge. Each object is accompanied
 // by it's size to avoid dereferencing a map pointer for scanning.
@@ -779,8 +778,11 @@ class Heap {
 
   // An object should be promoted if the object has survived a
   // scavenge operation.
+  template <PromotionMode promotion_mode>
   inline bool ShouldBePromoted(Address old_address, int object_size);
 
+  inline PromotionMode CurrentPromotionMode();
+
   void ClearNormalizedMapCaches();
 
   void IncrementDeferredCount(v8::Isolate::UseCounterFeature feature);
@@ -1698,7 +1700,8 @@ class Heap {
   // Performs a minor collection in new generation.
   void Scavenge();
 
-  Address DoScavenge(ObjectVisitor* scavenge_visitor, Address new_space_front);
+  Address DoScavenge(ObjectVisitor* scavenge_visitor, Address new_space_front,
+                     PromotionMode promotion_mode);
 
   void UpdateNewSpaceReferencesInExternalStringTable(
       ExternalStringTableUpdaterCallback updater_func);

src/heap/mark-compact.cc

@@ -1747,7 +1747,7 @@ class MarkCompactCollector::EvacuateNewSpaceVisitor final
                                                local_pretenuring_feedback_);
     int size = object->Size();
     HeapObject* target_object = nullptr;
-    if (heap_->ShouldBePromoted(object->address(), size) &&
+    if (heap_->ShouldBePromoted<DEFAULT_PROMOTION>(object->address(), size) &&
         TryEvacuateObject(compaction_spaces_->Get(OLD_SPACE), object,
                           &target_object)) {
       // If we end up needing more special cases, we should factor this out.

src/heap/scavenger-inl.h

@@ -10,8 +10,7 @@
 namespace v8 {
 namespace internal {
 
-void Scavenger::ScavengeObject(HeapObject** p, HeapObject* object,
-                               PromotionMode promotion_mode) {
+void Scavenger::ScavengeObject(HeapObject** p, HeapObject* object) {
   DCHECK(object->GetIsolate()->heap()->InFromSpace(object));
 
   // We use the first word (where the map pointer usually is) of a heap
@@ -35,19 +34,18 @@ void Scavenger::ScavengeObject(HeapObject** p, HeapObject* object,
   // AllocationMementos are unrooted and shouldn't survive a scavenge
   DCHECK(object->map() != object->GetHeap()->allocation_memento_map());
   // Call the slow part of scavenge object.
-  return ScavengeObjectSlow(p, object, promotion_mode);
+  return ScavengeObjectSlow(p, object);
 }
 
-SlotCallbackResult Scavenger::CheckAndScavengeObject(
-    Heap* heap, Address slot_address, PromotionMode promotion_mode) {
+SlotCallbackResult Scavenger::CheckAndScavengeObject(Heap* heap,
+                                                     Address slot_address) {
   Object** slot = reinterpret_cast<Object**>(slot_address);
   Object* object = *slot;
   if (heap->InFromSpace(object)) {
     HeapObject* heap_object = reinterpret_cast<HeapObject*>(object);
     DCHECK(heap_object->IsHeapObject());
 
-    ScavengeObject(reinterpret_cast<HeapObject**>(slot), heap_object,
-                   promotion_mode);
+    ScavengeObject(reinterpret_cast<HeapObject**>(slot), heap_object);
 
     object = *slot;
     // If the object was in from space before and is after executing the
@@ -64,13 +62,14 @@ SlotCallbackResult Scavenger::CheckAndScavengeObject(
 }
 
 // static
-void StaticScavengeVisitor::VisitPointer(Heap* heap, HeapObject* obj,
+template <PromotionMode promotion_mode>
+void StaticScavengeVisitor<promotion_mode>::VisitPointer(Heap* heap,
+                                                         HeapObject* obj,
                                                          Object** p) {
   Object* object = *p;
   if (!heap->InNewSpace(object)) return;
   Scavenger::ScavengeObject(reinterpret_cast<HeapObject**>(p),
-                            reinterpret_cast<HeapObject*>(object),
-                            DEFAULT_PROMOTION);
+                            reinterpret_cast<HeapObject*>(object));
 }
 
 }  // namespace internal

src/heap/scavenger.h

@@ -12,8 +12,7 @@ namespace v8 {
 namespace internal {
 
 typedef void (*ScavengingCallback)(Map* map, HeapObject** slot,
-                                   HeapObject* object,
-                                   PromotionMode promotion_mode);
+                                   HeapObject* object);
 
 class Scavenger {
  public:
@@ -26,15 +25,12 @@ class Scavenger {
   // necessary, the object might be promoted to an old space.  The caller must
   // ensure the precondition that the object is (a) a heap object and (b) in
   // the heap's from space.
-  static inline void ScavengeObject(
-      HeapObject** p, HeapObject* object,
-      PromotionMode promotion_mode = DEFAULT_PROMOTION);
-  static inline SlotCallbackResult CheckAndScavengeObject(
-      Heap* heap, Address slot_address, PromotionMode promotion_mode);
+  static inline void ScavengeObject(HeapObject** p, HeapObject* object);
+  static inline SlotCallbackResult CheckAndScavengeObject(Heap* heap,
+                                                          Address slot_address);
 
   // Slow part of {ScavengeObject} above.
-  static void ScavengeObjectSlow(HeapObject** p, HeapObject* object,
-                                 PromotionMode promotion_mode);
+  static void ScavengeObjectSlow(HeapObject** p, HeapObject* object);
 
   // Chooses an appropriate static visitor table depending on the current state
   // of the heap (i.e. incremental marking, logging and profiling).
@@ -67,8 +63,9 @@ class ScavengeVisitor : public ObjectVisitor {
 
 // Helper class for turning the scavenger into an object visitor that is also
 // filtering out non-HeapObjects and objects which do not reside in new space.
+template <PromotionMode promotion_mode>
 class StaticScavengeVisitor
-    : public StaticNewSpaceVisitor<StaticScavengeVisitor> {
+    : public StaticNewSpaceVisitor<StaticScavengeVisitor<promotion_mode>> {
  public:
   static inline void VisitPointer(Heap* heap, HeapObject* object, Object** p);
 };

test/cctest/heap/test-heap.cc

@@ -711,6 +711,44 @@ TEST(DeleteWeakGlobalHandle) {
   CHECK(WeakPointerCleared);
 }
 
+TEST(DoNotPromoteWhiteObjectsOnScavenge) {
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+  Factory* factory = isolate->factory();
+
+  HandleScope scope(isolate);
+  Handle<Object> white = factory->NewStringFromStaticChars("white");
+
+  CHECK(Marking::IsWhite(Marking::MarkBitFrom(HeapObject::cast(*white))));
+
+  heap->CollectGarbage(NEW_SPACE);
+
+  CHECK(heap->InNewSpace(*white));
+}
+
+TEST(PromoteGreyOrBlackObjectsOnScavenge) {
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+  Factory* factory = isolate->factory();
+
+  HandleScope scope(isolate);
+  Handle<Object> marked = factory->NewStringFromStaticChars("marked");
+
+  IncrementalMarking* marking = heap->incremental_marking();
+  marking->Stop();
+  heap->StartIncrementalMarking();
+  while (Marking::IsWhite(Marking::MarkBitFrom(HeapObject::cast(*marked)))) {
+    marking->Step(MB, IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+                  IncrementalMarking::FORCE_MARKING,
+                  IncrementalMarking::DO_NOT_FORCE_COMPLETION);
+  }
+
+  heap->CollectGarbage(NEW_SPACE);
+
+  CHECK(!heap->InNewSpace(*marked));
+}
 
 TEST(BytecodeArray) {
   static const uint8_t kRawBytes[] = {0xc3, 0x7e, 0xa5, 0x5a};