[heap] Don't store host object offset for typed slots

because RelocInfo does not need host Code object for updating pointers to heap
objects embedded into code.

This CL also simplifies typed slot iteration callback signature.

Bug: v8:8518, v8:8262
Change-Id: I59fe9e3b4e9b69e3d87b5449c80bed14e311516f
Reviewed-on: https://chromium-review.googlesource.com/c/1370037Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58136}

src/heap/concurrent-marking.cc

@@ -519,15 +519,15 @@ class ConcurrentMarkingVisitor final
     return slot_snapshot_;
   }
 
-  void RecordRelocSlot(Code host, RelocInfo* rinfo, Object* target) {
-    auto info =
+  void RecordRelocSlot(Code host, RelocInfo* rinfo, HeapObject* target) {
+    MarkCompactCollector::RecordRelocSlotInfo info =
         MarkCompactCollector::PrepareRecordRelocSlot(host, rinfo, target);
     if (info.should_record) {
       MemoryChunkData& data = (*memory_chunk_data_)[info.memory_chunk];
       if (!data.typed_slots) {
         data.typed_slots.reset(new TypedSlots());
       }
-      data.typed_slots->Insert(info.slot_type, info.host_offset, info.offset);
+      data.typed_slots->Insert(info.slot_type, info.offset);
     }
   }
 

src/heap/heap.cc

@@ -3621,7 +3621,7 @@ void CollectSlots(MemoryChunk* chunk, Address start, Address end,
       },
       SlotSet::PREFREE_EMPTY_BUCKETS);
   RememberedSet<direction>::IterateTyped(
-      chunk, [start, end, typed](SlotType type, Address host, Address slot) {
+      chunk, [=](SlotType type, Address slot) {
         if (start <= slot && slot < end) {
           typed->insert(std::make_pair(type, slot));
         }
@@ -5628,13 +5628,9 @@ void Heap::GenerationalBarrierForCodeSlow(Code host, RelocInfo* rinfo,
       slot_type = OBJECT_SLOT;
     }
   }
-  Address host_addr = host.ptr();
   uintptr_t offset = addr - source_page->address();
-  uintptr_t host_offset = host_addr - source_page->address();
   DCHECK_LT(offset, static_cast<uintptr_t>(TypedSlotSet::kMaxOffset));
-  DCHECK_LT(host_offset, static_cast<uintptr_t>(TypedSlotSet::kMaxOffset));
   RememberedSet<OLD_TO_NEW>::InsertTyped(source_page, slot_type,
-                                         static_cast<uint32_t>(host_offset),
                                          static_cast<uint32_t>(offset));
 }
 

src/heap/mark-compact.cc

@@ -2225,10 +2225,10 @@ bool MarkCompactCollector::IsOnEvacuationCandidate(MaybeObject obj) {
 
 MarkCompactCollector::RecordRelocSlotInfo
 MarkCompactCollector::PrepareRecordRelocSlot(Code host, RelocInfo* rinfo,
-                                             Object* target) {
+                                             HeapObject* target) {
   RecordRelocSlotInfo result;
   result.should_record = false;
-  Page* target_page = Page::FromAddress(reinterpret_cast<Address>(target));
+  Page* target_page = Page::FromAddress(target->ptr());
   Page* source_page = Page::FromAddress(host.ptr());
   if (target_page->IsEvacuationCandidate() &&
       (rinfo->host().is_null() ||
@@ -2245,26 +2245,22 @@ MarkCompactCollector::PrepareRecordRelocSlot(Code host, RelocInfo* rinfo,
         slot_type = OBJECT_SLOT;
       }
     }
-    Address host_addr = host.ptr();
     uintptr_t offset = addr - source_page->address();
-    uintptr_t host_offset = host_addr - source_page->address();
     DCHECK_LT(offset, static_cast<uintptr_t>(TypedSlotSet::kMaxOffset));
-    DCHECK_LT(host_offset, static_cast<uintptr_t>(TypedSlotSet::kMaxOffset));
     result.should_record = true;
     result.memory_chunk = source_page;
     result.slot_type = slot_type;
-    result.host_offset = static_cast<uint32_t>(host_offset);
     result.offset = static_cast<uint32_t>(offset);
   }
   return result;
 }
 
 void MarkCompactCollector::RecordRelocSlot(Code host, RelocInfo* rinfo,
-                                           Object* target) {
-  auto info = PrepareRecordRelocSlot(host, rinfo, target);
+                                           HeapObject* target) {
+  RecordRelocSlotInfo info = PrepareRecordRelocSlot(host, rinfo, target);
   if (info.should_record) {
     RememberedSet<OLD_TO_OLD>::InsertTyped(info.memory_chunk, info.slot_type,
-                                           info.host_offset, info.offset);
+                                           info.offset);
   }
 }
 
@@ -2344,7 +2340,6 @@ static inline SlotCallbackResult UpdateSlot(TSlot slot,
   return REMOVE_SLOT;
 }
 
-// TODO(ishell): TSlot-ify this function
 template <AccessMode access_mode, typename TSlot>
 static inline SlotCallbackResult UpdateSlot(TSlot slot) {
   typename TSlot::TObject obj = slot.Relaxed_Load();
@@ -3214,7 +3209,7 @@ class RememberedSetUpdatingItem : public UpdatingItem {
             return CheckAndUpdateOldToNewSlot(slot);
           };
       RememberedSet<OLD_TO_NEW>::IterateTyped(
-          chunk_, [=](SlotType slot_type, Address host_addr, Address slot) {
+          chunk_, [=](SlotType slot_type, Address slot) {
             return UpdateTypedSlotHelper::UpdateTypedSlot(
                 heap_, slot_type, slot, check_and_update_old_to_new_slot_fn);
           });
@@ -3224,7 +3219,7 @@ class RememberedSetUpdatingItem : public UpdatingItem {
          nullptr)) {
       CHECK_NE(chunk_->owner(), heap_->map_space());
       RememberedSet<OLD_TO_OLD>::IterateTyped(
-          chunk_, [this](SlotType slot_type, Address host_addr, Address slot) {
+          chunk_, [=](SlotType slot_type, Address slot) {
             // Using UpdateStrongSlot is OK here, because there are no weak
             // typed slots.
             return UpdateTypedSlotHelper::UpdateTypedSlot(
@@ -4381,8 +4376,7 @@ class PageMarkingItem : public MarkingItem {
 
   void MarkTypedPointers(YoungGenerationMarkingTask* task) {
     RememberedSet<OLD_TO_NEW>::IterateTyped(
-        chunk_,
-        [this, task](SlotType slot_type, Address host_addr, Address slot) {
+        chunk_, [=](SlotType slot_type, Address slot) {
           return UpdateTypedSlotHelper::UpdateTypedSlot(
               heap(), slot_type, slot, [this, task](FullMaybeObjectSlot slot) {
                 return CheckAndMarkObject(task, slot);

src/heap/mark-compact.h

@@ -626,15 +626,14 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
   static bool IsOnEvacuationCandidate(MaybeObject obj);
 
   struct RecordRelocSlotInfo {
-    bool should_record;
     MemoryChunk* memory_chunk;
     SlotType slot_type;
-    uint32_t host_offset;
+    bool should_record;
     uint32_t offset;
   };
   static RecordRelocSlotInfo PrepareRecordRelocSlot(Code host, RelocInfo* rinfo,
-                                                    Object* target);
-  static void RecordRelocSlot(Code host, RelocInfo* rinfo, Object* target);
+                                                    HeapObject* target);
+  static void RecordRelocSlot(Code host, RelocInfo* rinfo, HeapObject* target);
   V8_INLINE static void RecordSlot(HeapObject* object, ObjectSlot slot,
                                    HeapObject* target);
   V8_INLINE static void RecordSlot(HeapObject* object, HeapObjectSlot slot,

src/heap/remembered-set.h

@@ -191,13 +191,12 @@ class RememberedSet : public AllStatic {
   // Given a page and a typed slot in that page, this function adds the slot
   // to the remembered set.
   static void InsertTyped(MemoryChunk* memory_chunk, SlotType slot_type,
-                          uint32_t host_offset, uint32_t offset) {
+                          uint32_t offset) {
     TypedSlotSet* slot_set = memory_chunk->typed_slot_set<type>();
     if (slot_set == nullptr) {
       slot_set = memory_chunk->AllocateTypedSlotSet<type>();
     }
-    slot_set->Insert(slot_type, static_cast<uint32_t>(host_offset),
-                     static_cast<uint32_t>(offset));
+    slot_set->Insert(slot_type, offset);
   }
 
   static void MergeTyped(MemoryChunk* page, std::unique_ptr<TypedSlots> slots) {
@@ -214,8 +213,7 @@ class RememberedSet : public AllStatic {
     TypedSlotSet* slots = page->typed_slot_set<type>();
     if (slots != nullptr) {
       slots->Iterate(
-          [start, end](SlotType slot_type, Address host_addr,
-                       Address slot_addr) {
+          [=](SlotType slot_type, Address slot_addr) {
             return start <= slot_addr && slot_addr < end ? REMOVE_SLOT
                                                          : KEEP_SLOT;
           },
@@ -224,7 +222,7 @@ class RememberedSet : public AllStatic {
   }
 
   // Iterates and filters the remembered set with the given callback.
-  // The callback should take (SlotType slot_type, SlotAddress slot) and return
+  // The callback should take (SlotType slot_type, Address addr) and return
   // SlotCallbackResult.
   template <typename Callback>
   static void IterateTyped(Heap* heap, RememberedSetIterationMode mode,
@@ -238,7 +236,7 @@ class RememberedSet : public AllStatic {
 
   // Iterates and filters typed old to old pointers in the given memory chunk
   // with the given callback. The callback should take (SlotType slot_type,
-  // Address slot_addr) and return SlotCallbackResult.
+  // Address addr) and return SlotCallbackResult.
   template <typename Callback>
   static void IterateTyped(MemoryChunk* chunk, Callback callback) {
     TypedSlotSet* slots = chunk->typed_slot_set<type>();
@@ -298,13 +296,12 @@ class UpdateTypedSlotHelper {
 
  private:
   // Updates a code entry slot using an untyped slot callback.
-  // The callback accepts MaybeObjectSlot and returns SlotCallbackResult.
+  // The callback accepts FullMaybeObjectSlot and returns SlotCallbackResult.
   template <typename Callback>
   static SlotCallbackResult UpdateCodeEntry(Address entry_address,
                                             Callback callback) {
     Code code = Code::GetObjectFromEntryAddress(entry_address);
     Code old_code = code;
-    STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
     SlotCallbackResult result = callback(FullMaybeObjectSlot(&code));
     DCHECK(!HasWeakHeapObjectTag(code.ptr()));
     if (code != old_code) {
@@ -314,14 +311,13 @@ class UpdateTypedSlotHelper {
   }
 
   // Updates a code target slot using an untyped slot callback.
-  // The callback accepts MaybeObjectSlot and returns SlotCallbackResult.
+  // The callback accepts FullMaybeObjectSlot and returns SlotCallbackResult.
   template <typename Callback>
   static SlotCallbackResult UpdateCodeTarget(RelocInfo* rinfo,
                                              Callback callback) {
     DCHECK(RelocInfo::IsCodeTargetMode(rinfo->rmode()));
     Code old_target = Code::GetCodeFromTargetAddress(rinfo->target_address());
     Code new_target = old_target;
-    STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
     SlotCallbackResult result = callback(FullMaybeObjectSlot(&new_target));
     DCHECK(!HasWeakHeapObjectTag(new_target.ptr()));
     if (new_target != old_target) {
@@ -332,14 +328,13 @@ class UpdateTypedSlotHelper {
   }
 
   // Updates an embedded pointer slot using an untyped slot callback.
-  // The callback accepts MaybeObjectSlot and returns SlotCallbackResult.
+  // The callback accepts FullMaybeObjectSlot and returns SlotCallbackResult.
   template <typename Callback>
   static SlotCallbackResult UpdateEmbeddedPointer(Heap* heap, RelocInfo* rinfo,
                                                   Callback callback) {
     DCHECK(rinfo->rmode() == RelocInfo::EMBEDDED_OBJECT);
     HeapObject* old_target = rinfo->target_object();
     HeapObject* new_target = old_target;
-    STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
     SlotCallbackResult result = callback(FullMaybeObjectSlot(&new_target));
     DCHECK(!HasWeakHeapObjectTag(new_target));
     if (new_target != old_target) {

src/heap/scavenger.cc

@@ -387,7 +387,7 @@ void Scavenger::ScavengePage(MemoryChunk* page) {
                                      },
                                      SlotSet::KEEP_EMPTY_BUCKETS);
   RememberedSet<OLD_TO_NEW>::IterateTyped(
-      page, [this](SlotType type, Address host_addr, Address addr) {
+      page, [=](SlotType type, Address addr) {
         return UpdateTypedSlotHelper::UpdateTypedSlot(
             heap_, type, addr, [this](FullMaybeObjectSlot slot) {
               return CheckAndScavengeObject(heap(), slot);

src/heap/slot-set.cc

@@ -19,9 +19,8 @@ TypedSlots::~TypedSlots() {
   tail_ = nullptr;
 }
 
-void TypedSlots::Insert(SlotType type, uint32_t host_offset, uint32_t offset) {
-  TypedSlot slot = {TypeField::encode(type) | OffsetField::encode(offset),
-                    host_offset};
+void TypedSlots::Insert(SlotType type, uint32_t offset) {
+  TypedSlot slot = {TypeField::encode(type) | OffsetField::encode(offset)};
   Chunk* chunk = EnsureChunk();
   DCHECK_LT(chunk->count, chunk->capacity);
   chunk->buffer[chunk->count] = slot;
@@ -80,15 +79,15 @@ void TypedSlotSet::ClearInvalidSlots(
       TypedSlot slot = LoadTypedSlot(buffer + i);
       SlotType type = TypeField::decode(slot.type_and_offset);
       if (type == CLEARED_SLOT) continue;
-      uint32_t host_offset = slot.host_offset;
+      uint32_t offset = OffsetField::decode(slot.type_and_offset);
       std::map<uint32_t, uint32_t>::const_iterator upper_bound =
-          invalid_ranges.upper_bound(host_offset);
+          invalid_ranges.upper_bound(offset);
       if (upper_bound == invalid_ranges.begin()) continue;
       // upper_bounds points to the invalid range after the given slot. Hence,
       // we have to go to the previous element.
       upper_bound--;
-      DCHECK_LE(upper_bound->first, host_offset);
-      if (upper_bound->second > host_offset) {
+      DCHECK_LE(upper_bound->first, offset);
+      if (upper_bound->second > offset) {
         ClearTypedSlot(buffer + i);
       }
     }

src/heap/slot-set.h

@@ -404,8 +404,7 @@ class TypedSlots {
   static const int kMaxOffset = 1 << 29;
   TypedSlots() = default;
   virtual ~TypedSlots();
-  V8_EXPORT_PRIVATE void Insert(SlotType type, uint32_t host_offset,
-                                uint32_t offset);
+  V8_EXPORT_PRIVATE void Insert(SlotType type, uint32_t offset);
   V8_EXPORT_PRIVATE void Merge(TypedSlots* other);
 
  protected:
@@ -413,7 +412,6 @@ class TypedSlots {
   class TypeField : public BitField<SlotType, 29, 3> {};
   struct TypedSlot {
     uint32_t type_and_offset;
-    uint32_t host_offset;
   };
   struct Chunk {
     Chunk* next;
@@ -471,8 +469,7 @@ class V8_EXPORT_PRIVATE TypedSlotSet : public TypedSlots {
         if (type != CLEARED_SLOT) {
           uint32_t offset = OffsetField::decode(slot.type_and_offset);
           Address addr = page_start_ + offset;
-          Address host_addr = page_start_ + slot.host_offset;
-          if (callback(type, host_addr, addr) == KEEP_SLOT) {
+          if (callback(type, addr) == KEEP_SLOT) {
             new_count++;
             empty = false;
           } else {
@@ -519,20 +516,13 @@ class V8_EXPORT_PRIVATE TypedSlotSet : public TypedSlots {
     base::AsAtomicPointer::Relaxed_Store(&head_, chunk);
   }
   TypedSlot LoadTypedSlot(TypedSlot* slot) {
-    // Order is important here and should match that of ClearTypedSlot. The
-    // order guarantees that type != CLEARED_SLOT implies valid host_offset.
-    TypedSlot result;
-    result.host_offset = base::AsAtomic32::Acquire_Load(&slot->host_offset);
-    result.type_and_offset =
-        base::AsAtomic32::Relaxed_Load(&slot->type_and_offset);
-    return result;
+    return TypedSlot{base::AsAtomic32::Relaxed_Load(&slot->type_and_offset)};
   }
   void ClearTypedSlot(TypedSlot* slot) {
     // Order is important here and should match that of LoadTypedSlot.
     base::AsAtomic32::Relaxed_Store(
         &slot->type_and_offset,
         TypeField::encode(CLEARED_SLOT) | OffsetField::encode(0));
-    base::AsAtomic32::Release_Store(&slot->host_offset, 0);
   }
 
   Address page_start_;

test/unittests/heap/slot-set-unittest.cc

@@ -154,23 +154,18 @@ TEST(TypedSlotSet, Iterate) {
   // for a MSVC++ bug about lambda captures, see the discussion at
   // https://social.msdn.microsoft.com/Forums/SqlServer/4abf18bd-4ae4-4c72-ba3e-3b13e7909d5f
   static const int kDelta = 10000001;
-  static const int kHostDelta = 50001;
   int added = 0;
-  uint32_t j = 0;
-  for (uint32_t i = 0; i < TypedSlotSet::kMaxOffset;
-       i += kDelta, j += kHostDelta) {
+  for (uint32_t i = 0; i < TypedSlotSet::kMaxOffset; i += kDelta) {
     SlotType type = static_cast<SlotType>(i % CLEARED_SLOT);
-    set.Insert(type, j, i);
+    set.Insert(type, i);
     ++added;
   }
   int iterated = 0;
   set.Iterate(
-      [&iterated](SlotType type, Address host_addr, Address addr) {
+      [&iterated](SlotType type, Address addr) {
         uint32_t i = static_cast<uint32_t>(addr);
-        uint32_t j = static_cast<uint32_t>(host_addr);
         EXPECT_EQ(i % CLEARED_SLOT, static_cast<uint32_t>(type));
         EXPECT_EQ(0u, i % kDelta);
-        EXPECT_EQ(0u, j % kHostDelta);
         ++iterated;
         return i % 2 == 0 ? KEEP_SLOT : REMOVE_SLOT;
       },
@@ -178,7 +173,7 @@ TEST(TypedSlotSet, Iterate) {
   EXPECT_EQ(added, iterated);
   iterated = 0;
   set.Iterate(
-      [&iterated](SlotType type, Address host_addr, Address addr) {
+      [&iterated](SlotType type, Address addr) {
         uint32_t i = static_cast<uint32_t>(addr);
         EXPECT_EQ(0u, i % 2);
         ++iterated;
@@ -194,7 +189,7 @@ TEST(TypedSlotSet, ClearInvalidSlots) {
   uint32_t entries = 10;
   for (uint32_t i = 0; i < entries; i++) {
     SlotType type = static_cast<SlotType>(i % CLEARED_SLOT);
-    set.Insert(type, i * kHostDelta, i * kHostDelta);
+    set.Insert(type, i * kHostDelta);
   }
 
   std::map<uint32_t, uint32_t> invalid_ranges;
@@ -209,8 +204,8 @@ TEST(TypedSlotSet, ClearInvalidSlots) {
     uint32_t start = it->first;
     uint32_t end = it->second;
     set.Iterate(
-        [start, end](SlotType slot_type, Address host_addr, Address slot_addr) {
-          CHECK(host_addr < start || host_addr >= end);
+        [=](SlotType slot_type, Address slot_addr) {
+          CHECK(slot_addr < start || slot_addr >= end);
           return KEEP_SLOT;
         },
         TypedSlotSet::KEEP_EMPTY_CHUNKS);
@@ -221,18 +216,17 @@ TEST(TypedSlotSet, Merge) {
   TypedSlotSet set0(0), set1(0);
   static const uint32_t kEntries = 10000;
   for (uint32_t i = 0; i < kEntries; i++) {
-    set0.Insert(EMBEDDED_OBJECT_SLOT, 2 * i, 2 * i);
-    set1.Insert(EMBEDDED_OBJECT_SLOT, 2 * i + 1, 2 * i + 1);
+    set0.Insert(EMBEDDED_OBJECT_SLOT, 2 * i);
+    set1.Insert(EMBEDDED_OBJECT_SLOT, 2 * i + 1);
   }
   uint32_t count = 0;
   set0.Merge(&set1);
   set0.Iterate(
-      [&count](SlotType slot_type, Address host_addr, Address slot_addr) {
-        CHECK_EQ(host_addr, slot_addr);
+      [&count](SlotType slot_type, Address slot_addr) {
         if (count < kEntries) {
-          CHECK_EQ(host_addr % 2, 0);
+          CHECK_EQ(slot_addr % 2, 0);
         } else {
-          CHECK_EQ(host_addr % 2, 1);
+          CHECK_EQ(slot_addr % 2, 1);
         }
         ++count;
         return KEEP_SLOT;
@@ -240,7 +234,7 @@ TEST(TypedSlotSet, Merge) {
       TypedSlotSet::KEEP_EMPTY_CHUNKS);
   CHECK_EQ(2 * kEntries, count);
   set1.Iterate(
-      [](SlotType slot_type, Address host_addr, Address slot_addr) {
+      [](SlotType slot_type, Address slot_addr) {
         CHECK(false);  // Unreachable.
         return KEEP_SLOT;
       },