[heap] Simplify Sweeper::CleanupInvalidTypedSlotsOfFreeRanges

This CL only refactors code in the sweeper without changing behavior.

This method can be simplified by moving duplicate code into new methods.
Also move definition of FreeRangesMap into TypedSlotSet and replace all
usages of that raw map type with that type-alias.

Since we are already here, remove the unused argument in
Sweeper::FreeAndProcessFreedMemory.

Bug: v8:12760
Change-Id: Ifa1848b456aef7955eccbaafc00df55fbcbc385c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3574542Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79822}

src/heap/slot-set.cc

@@ -61,14 +61,12 @@ TypedSlots::Chunk* TypedSlots::NewChunk(Chunk* next, size_t capacity) {
   return chunk;
 }
 
-void TypedSlotSet::ClearInvalidSlots(
-    const std::map<uint32_t, uint32_t>& invalid_ranges) {
+void TypedSlotSet::ClearInvalidSlots(const FreeRangesMap& invalid_ranges) {
   IterateSlotsInRanges([](TypedSlot* slot) { *slot = ClearedTypedSlot(); },
                        invalid_ranges);
 }
 
-void TypedSlotSet::AssertNoInvalidSlots(
-    const std::map<uint32_t, uint32_t>& invalid_ranges) {
+void TypedSlotSet::AssertNoInvalidSlots(const FreeRangesMap& invalid_ranges) {
   IterateSlotsInRanges(
       [](TypedSlot* slot) {
         CHECK_WITH_MSG(false, "No slot in ranges expected.");
@@ -77,8 +75,8 @@ void TypedSlotSet::AssertNoInvalidSlots(
 }
 
 template <typename Callback>
-void TypedSlotSet::IterateSlotsInRanges(
-    Callback callback, const std::map<uint32_t, uint32_t>& ranges) {
+void TypedSlotSet::IterateSlotsInRanges(Callback callback,
+                                        const FreeRangesMap& ranges) {
   if (ranges.empty()) return;
 
   Chunk* chunk = LoadHead();
@@ -87,8 +85,7 @@ void TypedSlotSet::IterateSlotsInRanges(
       SlotType type = TypeField::decode(slot.type_and_offset);
       if (type == SlotType::kCleared) continue;
       uint32_t offset = OffsetField::decode(slot.type_and_offset);
-      std::map<uint32_t, uint32_t>::const_iterator upper_bound =
-          ranges.upper_bound(offset);
+      FreeRangesMap::const_iterator upper_bound = ranges.upper_bound(offset);
       if (upper_bound == ranges.begin()) continue;
       // upper_bounds points to the invalid range after the given slot. Hence,
       // we have to go to the previous element.

src/heap/slot-set.h

@@ -681,6 +681,8 @@ class V8_EXPORT_PRIVATE TypedSlots {
 // clearing of invalid slots.
 class V8_EXPORT_PRIVATE TypedSlotSet : public TypedSlots {
  public:
+  using FreeRangesMap = std::map<uint32_t, uint32_t>;
+
   enum IterationMode { FREE_EMPTY_CHUNKS, KEEP_EMPTY_CHUNKS };
 
   explicit TypedSlotSet(Address page_start) : page_start_(page_start) {}
@@ -737,10 +739,10 @@ class V8_EXPORT_PRIVATE TypedSlotSet : public TypedSlots {
 
   // Clears all slots that have the offset in the specified ranges.
   // This can run concurrently to Iterate().
-  void ClearInvalidSlots(const std::map<uint32_t, uint32_t>& invalid_ranges);
+  void ClearInvalidSlots(const FreeRangesMap& invalid_ranges);
 
   // Asserts that there are no recorded slots in the specified ranges.
-  void AssertNoInvalidSlots(const std::map<uint32_t, uint32_t>& invalid_ranges);
+  void AssertNoInvalidSlots(const FreeRangesMap& invalid_ranges);
 
   // Frees empty chunks accumulated by PREFREE_EMPTY_CHUNKS.
   void FreeToBeFreedChunks();
@@ -748,7 +750,7 @@ class V8_EXPORT_PRIVATE TypedSlotSet : public TypedSlots {
  private:
   template <typename Callback>
   void IterateSlotsInRanges(Callback callback,
-                            const std::map<uint32_t, uint32_t>& invalid_ranges);
+                            const FreeRangesMap& invalid_ranges);
 
   // Atomic operations used by Iterate and ClearInvalidSlots;
   Chunk* LoadNext(Chunk* chunk) {

src/heap/spaces.h

@@ -21,6 +21,7 @@
 #include "src/heap/list.h"
 #include "src/heap/memory-chunk-layout.h"
 #include "src/heap/memory-chunk.h"
+#include "src/heap/slot-set.h"
 #include "src/objects/objects.h"
 #include "src/utils/allocation.h"
 #include "src/utils/utils.h"
@@ -311,6 +312,24 @@ class Page : public MemoryChunk {
 
   ActiveSystemPages* active_system_pages() { return &active_system_pages_; }
 
+  template <RememberedSetType remembered_set>
+  void ClearInvalidTypedSlots(const TypedSlotSet::FreeRangesMap& ranges) {
+    TypedSlotSet* typed_slot_set = this->typed_slot_set<remembered_set>();
+    if (typed_slot_set != nullptr) {
+      typed_slot_set->ClearInvalidSlots(ranges);
+    }
+  }
+
+  template <RememberedSetType remembered_set>
+  void AssertNoInvalidTypedSlots(const TypedSlotSet::FreeRangesMap& ranges) {
+#if DEBUG
+    TypedSlotSet* typed_slot_set = this->typed_slot_set<OLD_TO_OLD>();
+    if (typed_slot_set != nullptr) {
+      typed_slot_set->AssertNoInvalidSlots(ranges);
+    }
+#endif  // DEBUG
+  }
+
  private:
   friend class MemoryAllocator;
 };

src/heap/sweeper.cc

@@ -227,7 +227,7 @@ bool Sweeper::AreSweeperTasksRunning() {
 
 V8_INLINE size_t Sweeper::FreeAndProcessFreedMemory(
     Address free_start, Address free_end, Page* page, Space* space,
-    bool record_free_ranges, FreeListRebuildingMode free_list_mode,
+    FreeListRebuildingMode free_list_mode,
     FreeSpaceTreatmentMode free_space_mode) {
   CHECK_GT(free_end, free_start);
   size_t freed_bytes = 0;
@@ -252,7 +252,7 @@ V8_INLINE size_t Sweeper::FreeAndProcessFreedMemory(
 
 V8_INLINE void Sweeper::CleanupRememberedSetEntriesForFreedMemory(
     Address free_start, Address free_end, Page* page, bool record_free_ranges,
-    FreeRangesMap* free_ranges_map, SweepingMode sweeping_mode,
+    TypedSlotSet::FreeRangesMap* free_ranges_map, SweepingMode sweeping_mode,
     InvalidatedSlotsCleanup* old_to_new_cleanup) {
   DCHECK_LE(free_start, free_end);
   if (sweeping_mode == SweepingMode::kEagerDuringGC) {
@@ -282,37 +282,23 @@ V8_INLINE void Sweeper::CleanupRememberedSetEntriesForFreedMemory(
 }
 
 void Sweeper::CleanupInvalidTypedSlotsOfFreeRanges(
-    Page* page, const FreeRangesMap& free_ranges_map,
+    Page* page, const TypedSlotSet::FreeRangesMap& free_ranges_map,
     SweepingMode sweeping_mode) {
   if (sweeping_mode == SweepingMode::kEagerDuringGC) {
-    TypedSlotSet* old_to_new = page->typed_slot_set<OLD_TO_NEW>();
-    if (old_to_new != nullptr) {
-      old_to_new->ClearInvalidSlots(free_ranges_map);
-    }
+    page->ClearInvalidTypedSlots<OLD_TO_NEW>(free_ranges_map);
 
-#if DEBUG
-    TypedSlotSet* old_to_old = page->typed_slot_set<OLD_TO_OLD>();
-    if (old_to_old != nullptr) {
-      // Typed old-to-old slot sets are only ever recorded in live code objects.
-      // Also code objects are never right-trimmed, so there cannot be any slots
-      // in a free range.
-      old_to_old->AssertNoInvalidSlots(free_ranges_map);
-    }
-#endif  // DEBUG
+    // Typed old-to-old slot sets are only ever recorded in live code objects.
+    // Also code objects are never right-trimmed, so there cannot be any slots
+    // in a free range.
+    page->AssertNoInvalidTypedSlots<OLD_TO_OLD>(free_ranges_map);
     return;
   }
 
   DCHECK_EQ(sweeping_mode, SweepingMode::kLazyOrConcurrent);
 
-#if DEBUG
-  TypedSlotSet* old_to_new = page->typed_slot_set<OLD_TO_NEW>();
-  if (old_to_new != nullptr) {
-    // After a full GC there are no old-to-new typed slots. The main thread
-    // could create new slots but not in a free range.
-    old_to_new->AssertNoInvalidSlots(free_ranges_map);
-  }
-#endif  // DEBUG
-
+  // After a full GC there are no old-to-new typed slots. The main thread
+  // could create new slots but not in a free range.
+  page->AssertNoInvalidTypedSlots<OLD_TO_NEW>(free_ranges_map);
   DCHECK_NULL(page->typed_slot_set<OLD_TO_OLD>());
 }
 
@@ -380,7 +366,7 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
     old_to_new_cleanup = InvalidatedSlotsCleanup::OldToNew(p);
 
   // The free ranges map is used for filtering typed slots.
-  FreeRangesMap free_ranges_map;
+  TypedSlotSet::FreeRangesMap free_ranges_map;
 
 #ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
   p->object_start_bitmap()->Clear();
@@ -401,8 +387,7 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
       max_freed_bytes =
           std::max(max_freed_bytes,
                    FreeAndProcessFreedMemory(free_start, free_end, p, space,
-                                             record_free_ranges, free_list_mode,
-                                             free_space_mode));
+                                             free_list_mode, free_space_mode));
       CleanupRememberedSetEntriesForFreedMemory(
           free_start, free_end, p, record_free_ranges, &free_ranges_map,
           sweeping_mode, &old_to_new_cleanup);
@@ -428,10 +413,10 @@ int Sweeper::RawSweep(Page* p, FreeListRebuildingMode free_list_mode,
   // If there is free memory after the last live object also free that.
   Address free_end = p->area_end();
   if (free_end != free_start) {
-    max_freed_bytes = std::max(
-        max_freed_bytes, FreeAndProcessFreedMemory(
-                             free_start, free_end, p, space, record_free_ranges,
-                             free_list_mode, free_space_mode));
+    max_freed_bytes =
+        std::max(max_freed_bytes,
+                 FreeAndProcessFreedMemory(free_start, free_end, p, space,
+                                           free_list_mode, free_space_mode));
     CleanupRememberedSetEntriesForFreedMemory(
         free_start, free_end, p, record_free_ranges, &free_ranges_map,
         sweeping_mode, &old_to_new_cleanup);

src/heap/sweeper.h

@@ -11,6 +11,7 @@
 #include "src/base/platform/condition-variable.h"
 #include "src/base/platform/semaphore.h"
 #include "src/common/globals.h"
+#include "src/heap/slot-set.h"
 #include "src/tasks/cancelable-task.h"
 
 namespace v8 {
@@ -29,7 +30,6 @@ class Sweeper {
   using IterabilityList = std::vector<Page*>;
   using SweepingList = std::vector<Page*>;
   using SweptList = std::vector<Page*>;
-  using FreeRangesMap = std::map<uint32_t, uint32_t>;
 
   // Pauses the sweeper tasks.
   class V8_NODISCARD PauseScope final {
@@ -136,7 +136,6 @@ class Sweeper {
   // the operating system.
   size_t FreeAndProcessFreedMemory(Address free_start, Address free_end,
                                    Page* page, Space* space,
-                                   bool record_free_ranges,
                                    FreeListRebuildingMode free_list_mode,
                                    FreeSpaceTreatmentMode free_space_mode);
 
@@ -144,13 +143,13 @@ class Sweeper {
   // memory which require clearing.
   void CleanupRememberedSetEntriesForFreedMemory(
       Address free_start, Address free_end, Page* page, bool record_free_ranges,
-      FreeRangesMap* free_ranges_map, SweepingMode sweeping_mode,
+      TypedSlotSet::FreeRangesMap* free_ranges_map, SweepingMode sweeping_mode,
       InvalidatedSlotsCleanup* old_to_new_cleanup);
 
   // Helper function for RawSweep. Clears invalid typed slots in the given free
   // ranges.
   void CleanupInvalidTypedSlotsOfFreeRanges(
-      Page* page, const FreeRangesMap& free_ranges_map,
+      Page* page, const TypedSlotSet::FreeRangesMap& free_ranges_map,
       SweepingMode sweeping_mode);
 
   // Helper function for RawSweep. Clears the mark bits and ensures consistency

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

@@ -278,14 +278,14 @@ TEST(TypedSlotSet, ClearInvalidSlots) {
     set.Insert(type, i * kHostDelta);
   }
 
-  std::map<uint32_t, uint32_t> invalid_ranges;
+  TypedSlotSet::FreeRangesMap invalid_ranges;
   for (uint32_t i = 1; i < entries; i += 2) {
     invalid_ranges.insert(
         std::pair<uint32_t, uint32_t>(i * kHostDelta, i * kHostDelta + 1));
   }
 
   set.ClearInvalidSlots(invalid_ranges);
-  for (std::map<uint32_t, uint32_t>::iterator it = invalid_ranges.begin();
+  for (TypedSlotSet::FreeRangesMap::iterator it = invalid_ranges.begin();
        it != invalid_ranges.end(); ++it) {
     uint32_t start = it->first;
     uint32_t end = it->second;