[heap] Optimize handling of data only objects in Scavenger

This patch reduces the cost of the predicate that computes whether an
object contains only data or may contain pointers.

This also guards pushing to the copy_list_ with the predicate.

Bug: chromium:852420
Change-Id: I55c4e15eb8341708a21f484fb95b2c2cc2b25143
Reviewed-on: https://chromium-review.googlesource.com/c/1430068
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59074}

src/heap/heap.cc

@@ -5515,6 +5515,11 @@ void VerifyPointersVisitor::VerifyPointersImpl(TSlot start, TSlot end) {
 void VerifyPointersVisitor::VerifyPointers(HeapObject host,
                                            MaybeObjectSlot start,
                                            MaybeObjectSlot end) {
+  // If this DCHECK fires then you probably added a pointer field
+  // to one of objects in DATA_ONLY_VISITOR_ID_LIST. You can fix
+  // this by moving that object to POINTER_VISITOR_ID_LIST.
+  DCHECK_EQ(ObjectFields::kMaybePointers,
+            Map::ObjectFieldsFrom(host->map()->visitor_id()));
   VerifyPointersImpl(start, end);
 }
 

src/heap/objects-visiting-inl.h

@@ -59,6 +59,7 @@ ResultType HeapVisitor<ResultType, ConcreteVisitor>::Visit(Map map,
       return visitor->VisitFreeSpace(map, FreeSpace::cast(object));
     case kVisitWeakArray:
       return visitor->VisitWeakArray(map, object);
+    case kDataOnlyVisitorIdCount:
     case kVisitorIdCount:
       UNREACHABLE();
   }

src/heap/scavenger-inl.h

@@ -91,25 +91,6 @@ bool Scavenger::PromotionList::ShouldEagerlyProcessPromotionList(int task_id) {
   return LocalPushSegmentSize(task_id) < kProcessPromotionListThreshold;
 }
 
-// White list for objects that for sure only contain data.
-bool Scavenger::ContainsOnlyData(VisitorId visitor_id) {
-  switch (visitor_id) {
-    case kVisitSeqOneByteString:
-      return true;
-    case kVisitSeqTwoByteString:
-      return true;
-    case kVisitByteArray:
-      return true;
-    case kVisitFixedDoubleArray:
-      return true;
-    case kVisitDataObject:
-      return true;
-    default:
-      break;
-  }
-  return false;
-}
-
 void Scavenger::PageMemoryFence(MaybeObject object) {
 #ifdef THREAD_SANITIZER
   // Perform a dummy acquire load to tell TSAN that there is no data race
@@ -148,10 +129,9 @@ bool Scavenger::MigrateObject(Map map, HeapObject source, HeapObject target,
 }
 
 template <typename THeapObjectSlot>
-CopyAndForwardResult Scavenger::SemiSpaceCopyObject(Map map,
-                                                    THeapObjectSlot slot,
-                                                    HeapObject object,
-                                                    int object_size) {
+CopyAndForwardResult Scavenger::SemiSpaceCopyObject(
+    Map map, THeapObjectSlot slot, HeapObject object, int object_size,
+    ObjectFields object_fields) {
   static_assert(std::is_same<THeapObjectSlot, FullHeapObjectSlot>::value ||
                     std::is_same<THeapObjectSlot, HeapObjectSlot>::value,
                 "Only FullHeapObjectSlot and HeapObjectSlot are expected here");
@@ -175,8 +155,9 @@ CopyAndForwardResult Scavenger::SemiSpaceCopyObject(Map map,
                  : CopyAndForwardResult::SUCCESS_OLD_GENERATION;
     }
     HeapObjectReference::Update(slot, target);
-
-    copied_list_.Push(ObjectAndSize(target, object_size));
+    if (object_fields == ObjectFields::kMaybePointers) {
+      copied_list_.Push(ObjectAndSize(target, object_size));
+    }
     copied_size_ += object_size;
     return CopyAndForwardResult::SUCCESS_YOUNG_GENERATION;
   }
@@ -186,7 +167,8 @@ CopyAndForwardResult Scavenger::SemiSpaceCopyObject(Map map,
 template <typename THeapObjectSlot>
 CopyAndForwardResult Scavenger::PromoteObject(Map map, THeapObjectSlot slot,
                                               HeapObject object,
-                                              int object_size) {
+                                              int object_size,
+                                              ObjectFields object_fields) {
   static_assert(std::is_same<THeapObjectSlot, FullHeapObjectSlot>::value ||
                     std::is_same<THeapObjectSlot, HeapObjectSlot>::value,
                 "Only FullHeapObjectSlot and HeapObjectSlot are expected here");
@@ -209,7 +191,7 @@ CopyAndForwardResult Scavenger::PromoteObject(Map map, THeapObjectSlot slot,
                  : CopyAndForwardResult::SUCCESS_OLD_GENERATION;
     }
     HeapObjectReference::Update(slot, target);
-    if (!ContainsOnlyData(map->visitor_id())) {
+    if (object_fields == ObjectFields::kMaybePointers) {
       promotion_list_.PushRegularObject(target, object_size);
     }
     promoted_size_ += object_size;
@@ -225,7 +207,8 @@ SlotCallbackResult Scavenger::RememberedSetEntryNeeded(
                                                                   : REMOVE_SLOT;
 }
 
-bool Scavenger::HandleLargeObject(Map map, HeapObject object, int object_size) {
+bool Scavenger::HandleLargeObject(Map map, HeapObject object, int object_size,
+                                  ObjectFields object_fields) {
   // TODO(hpayer): Make this check size based, i.e.
   // object_size > kMaxRegularHeapObjectSize
   if (V8_UNLIKELY(
@@ -237,7 +220,7 @@ bool Scavenger::HandleLargeObject(Map map, HeapObject object, int object_size) {
             map, MapWord::FromForwardingAddress(object).ToMap()) == map) {
       surviving_new_large_objects_.insert({object, map});
 
-      if (!ContainsOnlyData(map->visitor_id())) {
+      if (object_fields == ObjectFields::kMaybePointers) {
         promotion_list_.PushLargeObject(object, map, object_size);
       }
     }
@@ -247,17 +230,16 @@ bool Scavenger::HandleLargeObject(Map map, HeapObject object, int object_size) {
 }
 
 template <typename THeapObjectSlot>
-SlotCallbackResult Scavenger::EvacuateObjectDefault(Map map,
-                                                    THeapObjectSlot slot,
-                                                    HeapObject object,
-                                                    int object_size) {
+SlotCallbackResult Scavenger::EvacuateObjectDefault(
+    Map map, THeapObjectSlot slot, HeapObject object, int object_size,
+    ObjectFields object_fields) {
   static_assert(std::is_same<THeapObjectSlot, FullHeapObjectSlot>::value ||
                     std::is_same<THeapObjectSlot, HeapObjectSlot>::value,
                 "Only FullHeapObjectSlot and HeapObjectSlot are expected here");
   SLOW_DCHECK(object->SizeFromMap(map) == object_size);
   CopyAndForwardResult result;
 
-  if (HandleLargeObject(map, object, object_size)) {
+  if (HandleLargeObject(map, object, object_size, object_fields)) {
     return REMOVE_SLOT;
   }
 
@@ -267,7 +249,7 @@ SlotCallbackResult Scavenger::EvacuateObjectDefault(Map map,
   if (!heap()->ShouldBePromoted(object->address())) {
     // A semi-space copy may fail due to fragmentation. In that case, we
     // try to promote the object.
-    result = SemiSpaceCopyObject(map, slot, object, object_size);
+    result = SemiSpaceCopyObject(map, slot, object, object_size, object_fields);
     if (result != CopyAndForwardResult::FAILURE) {
       return RememberedSetEntryNeeded(result);
     }
@@ -276,13 +258,13 @@ SlotCallbackResult Scavenger::EvacuateObjectDefault(Map map,
   // We may want to promote this object if the object was already semi-space
   // copied in a previes young generation GC or if the semi-space copy above
   // failed.
-  result = PromoteObject(map, slot, object, object_size);
+  result = PromoteObject(map, slot, object, object_size, object_fields);
   if (result != CopyAndForwardResult::FAILURE) {
     return RememberedSetEntryNeeded(result);
   }
 
   // If promotion failed, we try to copy the object to the other semi-space.
-  result = SemiSpaceCopyObject(map, slot, object, object_size);
+  result = SemiSpaceCopyObject(map, slot, object, object_size, object_fields);
   if (result != CopyAndForwardResult::FAILURE) {
     return RememberedSetEntryNeeded(result);
   }
@@ -310,7 +292,10 @@ SlotCallbackResult Scavenger::EvacuateThinString(Map map, THeapObjectSlot slot,
     return REMOVE_SLOT;
   }
 
-  return EvacuateObjectDefault(map, slot, object, object_size);
+  DCHECK_EQ(ObjectFields::kMaybePointers,
+            Map::ObjectFieldsFrom(map->visitor_id()));
+  return EvacuateObjectDefault(map, slot, object, object_size,
+                               ObjectFields::kMaybePointers);
 }
 
 template <typename THeapObjectSlot>
@@ -345,13 +330,16 @@ SlotCallbackResult Scavenger::EvacuateShortcutCandidate(Map map,
     }
     Map map = first_word.ToMap();
     SlotCallbackResult result =
-        EvacuateObjectDefault(map, slot, first, first->SizeFromMap(map));
+        EvacuateObjectDefault(map, slot, first, first->SizeFromMap(map),
+                              Map::ObjectFieldsFrom(map->visitor_id()));
     object->map_slot().Release_Store(
         MapWord::FromForwardingAddress(slot.ToHeapObject()).ToMap());
     return result;
   }
-
-  return EvacuateObjectDefault(map, slot, object, object_size);
+  DCHECK_EQ(ObjectFields::kMaybePointers,
+            Map::ObjectFieldsFrom(map->visitor_id()));
+  return EvacuateObjectDefault(map, slot, object, object_size,
+                               ObjectFields::kMaybePointers);
 }
 
 template <typename THeapObjectSlot>
@@ -365,7 +353,8 @@ SlotCallbackResult Scavenger::EvacuateObject(THeapObjectSlot slot, Map map,
   int size = source->SizeFromMap(map);
   // Cannot use ::cast() below because that would add checks in debug mode
   // that require re-reading the map.
-  switch (map->visitor_id()) {
+  VisitorId visitor_id = map->visitor_id();
+  switch (visitor_id) {
     case kVisitThinString:
       // At the moment we don't allow weak pointers to thin strings.
       DCHECK(!(*slot)->IsWeak());
@@ -377,7 +366,8 @@ SlotCallbackResult Scavenger::EvacuateObject(THeapObjectSlot slot, Map map,
       return EvacuateShortcutCandidate(
           map, slot, ConsString::unchecked_cast(source), size);
     default:
-      return EvacuateObjectDefault(map, slot, source, size);
+      return EvacuateObjectDefault(map, slot, source, size,
+                                   Map::ObjectFieldsFrom(visitor_id));
   }
 }
 

src/heap/scavenger.h

@@ -159,28 +159,28 @@ class Scavenger {
   RememberedSetEntryNeeded(CopyAndForwardResult result);
 
   template <typename THeapObjectSlot>
-  V8_INLINE CopyAndForwardResult SemiSpaceCopyObject(Map map,
-                                                     THeapObjectSlot slot,
-                                                     HeapObject object,
-                                                     int object_size);
+  V8_INLINE CopyAndForwardResult
+  SemiSpaceCopyObject(Map map, THeapObjectSlot slot, HeapObject object,
+                      int object_size, ObjectFields object_fields);
 
   template <typename THeapObjectSlot>
   V8_INLINE CopyAndForwardResult PromoteObject(Map map, THeapObjectSlot slot,
                                                HeapObject object,
-                                               int object_size);
+                                               int object_size,
+                                               ObjectFields object_fields);
 
   template <typename THeapObjectSlot>
   V8_INLINE SlotCallbackResult EvacuateObject(THeapObjectSlot slot, Map map,
                                               HeapObject source);
 
-  V8_INLINE bool HandleLargeObject(Map map, HeapObject object, int object_size);
+  V8_INLINE bool HandleLargeObject(Map map, HeapObject object, int object_size,
+                                   ObjectFields object_fields);
 
   // Different cases for object evacuation.
   template <typename THeapObjectSlot>
-  V8_INLINE SlotCallbackResult EvacuateObjectDefault(Map map,
-                                                     THeapObjectSlot slot,
-                                                     HeapObject object,
-                                                     int object_size);
+  V8_INLINE SlotCallbackResult
+  EvacuateObjectDefault(Map map, THeapObjectSlot slot, HeapObject object,
+                        int object_size, ObjectFields object_fields);
 
   template <typename THeapObjectSlot>
   inline SlotCallbackResult EvacuateThinString(Map map, THeapObjectSlot slot,
@@ -195,8 +195,6 @@ class Scavenger {
 
   void IterateAndScavengePromotedObject(HeapObject target, Map map, int size);
 
-  static inline bool ContainsOnlyData(VisitorId visitor_id);
-
   ScavengerCollector* const collector_;
   Heap* const heap_;
   PromotionList::View promotion_list_;

src/objects/map.h

@@ -18,10 +18,16 @@ namespace internal {
 
 enum InstanceType : uint16_t;
 
-#define VISITOR_ID_LIST(V)             \
+#define DATA_ONLY_VISITOR_ID_LIST(V) \
+  V(BigInt)                          \
+  V(ByteArray)                       \
+  V(DataObject)                      \
+  V(FixedDoubleArray)                \
+  V(SeqOneByteString)                \
+  V(SeqTwoByteString)
+
+#define POINTER_VISITOR_ID_LIST(V)     \
   V(AllocationSite)                    \
-  V(BigInt)                            \
-  V(ByteArray)                         \
   V(BytecodeArray)                     \
   V(Cell)                              \
   V(Code)                              \
@@ -29,14 +35,12 @@ enum InstanceType : uint16_t;
   V(ConsString)                        \
   V(Context)                           \
   V(DataHandler)                       \
-  V(DataObject)                        \
   V(DescriptorArray)                   \
   V(EmbedderDataArray)                 \
   V(EphemeronHashTable)                \
   V(FeedbackCell)                      \
   V(FeedbackVector)                    \
   V(FixedArray)                        \
-  V(FixedDoubleArray)                  \
   V(FixedFloat64Array)                 \
   V(FixedTypedArrayBase)               \
   V(FreeSpace)                         \
@@ -57,8 +61,6 @@ enum InstanceType : uint16_t;
   V(PropertyArray)                     \
   V(PropertyCell)                      \
   V(PrototypeInfo)                     \
-  V(SeqOneByteString)                  \
-  V(SeqTwoByteString)                  \
   V(SharedFunctionInfo)                \
   V(ShortcutCandidate)                 \
   V(SlicedString)                      \
@@ -74,22 +76,23 @@ enum InstanceType : uint16_t;
   V(WasmInstanceObject)                \
   V(WeakArray)
 
-// For data objects, JS objects and structs along with generic visitor which
-// can visit object of any size we provide visitors specialized by
-// object size in words.
-// Ids of specialized visitors are declared in a linear order (without
-// holes) starting from the id of visitor specialized for 2 words objects
-// (base visitor id) and ending with the id of generic visitor.
-// Method GetVisitorIdForSize depends on this ordering to calculate visitor
-// id of specialized visitor from given instance size, base visitor id and
-// generic visitor's id.
+// Objects with the same visitor id are processed in the same way by
+// the heap visitors. The visitor ids for data only objects must precede
+// other visitor ids. We rely on kDataOnlyVisitorIdCount for quick check
+// of whether an object contains only data or may contain pointers.
 enum VisitorId {
 #define VISITOR_ID_ENUM_DECL(id) kVisit##id,
-  VISITOR_ID_LIST(VISITOR_ID_ENUM_DECL)
+  DATA_ONLY_VISITOR_ID_LIST(VISITOR_ID_ENUM_DECL) kDataOnlyVisitorIdCount,
+  POINTER_VISITOR_ID_LIST(VISITOR_ID_ENUM_DECL)
 #undef VISITOR_ID_ENUM_DECL
       kVisitorIdCount
 };
 
+enum class ObjectFields {
+  kDataOnly,
+  kMaybePointers,
+};
+
 typedef std::vector<Handle<Map>> MapHandles;
 
 // All heap objects have a Map that describes their structure.
@@ -806,6 +809,12 @@ class Map : public HeapObject {
 
   DECL_PRIMITIVE_ACCESSORS(visitor_id, VisitorId)
 
+  static ObjectFields ObjectFieldsFrom(VisitorId visitor_id) {
+    return (visitor_id < kDataOnlyVisitorIdCount)
+               ? ObjectFields::kDataOnly
+               : ObjectFields::kMaybePointers;
+  }
+
   static Handle<Map> TransitionToPrototype(Isolate* isolate, Handle<Map> map,
                                            Handle<Object> prototype);