[heap] Reduce size of possibly empty buckets

Before this CL a byte was used per bucket to store whether the bucket
is possibly empty or not. This CL changes this such that each bucket
only needs a single bit.

PossiblyEmptyBuckets is now a word in the page header. If more bits
are needed than fit into a single word, an external bitmap is
allocated using AlignedAlloc. Storing this on the page header, allows
to remove initial_buckets from the SlotSet. The SlotSet allocation is
then again a power-of-2 in release mode.

Change-Id: If61fd5cfa153f98757beeb444a530f6e2803fdb6
Bug: chromium:1023139
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1906376
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64991}

src/heap/remembered-set.h

@@ -41,20 +41,6 @@ class RememberedSetOperations {
     return slots;
   }
 
-  template <typename Callback>
-  static int IterateAndTrackEmptyBuckets(
-      SlotSet* slot_set, MemoryChunk* chunk, Callback callback,
-      Worklist<MemoryChunk*, 64>::View empty_chunks) {
-    int slots = 0;
-    if (slot_set != nullptr) {
-      bool found_empty_bucket = false;
-      slots += slot_set->IterateAndTrackEmptyBuckets(
-          chunk->address(), chunk->buckets(), callback, &found_empty_bucket);
-      if (found_empty_bucket) empty_chunks.Push(chunk);
-    }
-    return slots;
-  }
-
   static void Remove(SlotSet* slot_set, MemoryChunk* chunk, Address slot_addr) {
     if (slot_set != nullptr) {
       uintptr_t offset = slot_addr - chunk->address();
@@ -168,12 +154,16 @@ class RememberedSet : public AllStatic {
   static int IterateAndTrackEmptyBuckets(
       MemoryChunk* chunk, Callback callback,
       Worklist<MemoryChunk*, 64>::View empty_chunks) {
-    SlotSet* slots = chunk->slot_set<type>();
-    bool empty_bucket_found = false;
-    int slot_count = RememberedSetOperations::IterateAndTrackEmptyBuckets(
-        slots, chunk, callback, empty_chunks);
-    if (empty_bucket_found) empty_chunks.Push(chunk);
-    return slot_count;
+    SlotSet* slot_set = chunk->slot_set<type>();
+    int slots = 0;
+    if (slot_set != nullptr) {
+      PossiblyEmptyBuckets* possibly_empty_buckets =
+          chunk->possibly_empty_buckets();
+      slots += slot_set->IterateAndTrackEmptyBuckets(
+          chunk->address(), chunk->buckets(), callback, possibly_empty_buckets);
+      if (!possibly_empty_buckets->IsEmpty()) empty_chunks.Push(chunk);
+    }
+    return slots;
   }
 
   static void FreeEmptyBuckets(MemoryChunk* chunk) {
@@ -188,7 +178,8 @@ class RememberedSet : public AllStatic {
     DCHECK(type == OLD_TO_NEW);
     SlotSet* slot_set = chunk->slot_set<type, AccessMode::NON_ATOMIC>();
     if (slot_set != nullptr &&
-        slot_set->CheckPossiblyEmptyBuckets(chunk->buckets())) {
+        slot_set->CheckPossiblyEmptyBuckets(chunk->buckets(),
+                                            chunk->possibly_empty_buckets())) {
       chunk->ReleaseSlotSet<type>();
       return true;
     }

src/heap/scavenger.cc

@@ -373,9 +373,7 @@ void ScavengerCollector::CollectGarbage() {
 #ifdef DEBUG
     RememberedSet<OLD_TO_NEW>::IterateMemoryChunks(
         heap_, [](MemoryChunk* chunk) {
-          SlotSet* slot_set = chunk->slot_set<OLD_TO_NEW>();
-          DCHECK_IMPLIES(slot_set != nullptr,
-                         slot_set->IsPossiblyEmptyCleared());
+          DCHECK(chunk->possibly_empty_buckets()->IsEmpty());
         });
 #endif
   }

src/heap/slot-set.h

@@ -23,6 +23,105 @@ namespace internal {
 
 enum SlotCallbackResult { KEEP_SLOT, REMOVE_SLOT };
 
+// Possibly empty buckets (buckets that do not contain any slots) are discovered
+// by the scavenger. Buckets might become non-empty when promoting objects later
+// or in another thread, so all those buckets need to be revisited.
+// Track possibly empty buckets within a SlotSet in this data structure. The
+// class contains a word-sized bitmap, in case more bits are needed the bitmap
+// is replaced with a pointer to a malloc-allocated bitmap.
+class PossiblyEmptyBuckets {
+ public:
+  PossiblyEmptyBuckets() : bitmap_(kNullAddress) {}
+  PossiblyEmptyBuckets(PossiblyEmptyBuckets&& other) V8_NOEXCEPT
+      : bitmap_(other.bitmap_) {
+    other.bitmap_ = kNullAddress;
+  }
+
+  ~PossiblyEmptyBuckets() { Release(); }
+
+  void Initialize() {
+    bitmap_ = kNullAddress;
+    DCHECK(!IsAllocated());
+  }
+
+  void Release() {
+    if (IsAllocated()) {
+      AlignedFree(BitmapArray());
+    }
+    bitmap_ = kNullAddress;
+    DCHECK(!IsAllocated());
+  }
+
+  void Insert(size_t bucket_index, size_t buckets) {
+    if (IsAllocated()) {
+      InsertAllocated(bucket_index);
+    } else if (bucket_index + 1 < kBitsPerWord) {
+      bitmap_ |= static_cast<uintptr_t>(1) << (bucket_index + 1);
+    } else {
+      Allocate(buckets);
+      InsertAllocated(bucket_index);
+    }
+  }
+
+  bool Contains(size_t bucket_index) {
+    if (IsAllocated()) {
+      size_t word_idx = bucket_index / kBitsPerWord;
+      uintptr_t* word = BitmapArray() + word_idx;
+      return *word &
+             (static_cast<uintptr_t>(1) << (bucket_index % kBitsPerWord));
+    } else if (bucket_index + 1 < kBitsPerWord) {
+      return bitmap_ & (static_cast<uintptr_t>(1) << (bucket_index + 1));
+    } else {
+      return false;
+    }
+  }
+
+  bool IsEmpty() { return bitmap_ == kNullAddress; }
+
+ private:
+  Address bitmap_;
+  static const Address kPointerTag = 1;
+  static const int kWordSize = sizeof(uintptr_t);
+  static const int kBitsPerWord = kWordSize * kBitsPerByte;
+
+  bool IsAllocated() { return bitmap_ & kPointerTag; }
+
+  void Allocate(size_t buckets) {
+    DCHECK(!IsAllocated());
+    size_t words = WordsForBuckets(buckets);
+    uintptr_t* ptr = reinterpret_cast<uintptr_t*>(
+        AlignedAlloc(words * kWordSize, kSystemPointerSize));
+    ptr[0] = bitmap_ >> 1;
+
+    for (size_t word_idx = 1; word_idx < words; word_idx++) {
+      ptr[word_idx] = 0;
+    }
+    bitmap_ = reinterpret_cast<Address>(ptr) + kPointerTag;
+    DCHECK(IsAllocated());
+  }
+
+  void InsertAllocated(size_t bucket_index) {
+    DCHECK(IsAllocated());
+    size_t word_idx = bucket_index / kBitsPerWord;
+    uintptr_t* word = BitmapArray() + word_idx;
+    *word |= static_cast<uintptr_t>(1) << (bucket_index % kBitsPerWord);
+  }
+
+  static size_t WordsForBuckets(size_t buckets) {
+    return (buckets / kBitsPerByte + kWordSize - 1) & ~(kWordSize - 1);
+  }
+
+  uintptr_t* BitmapArray() {
+    DCHECK(IsAllocated());
+    return reinterpret_cast<uintptr_t*>(bitmap_ & ~kPointerTag);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(PossiblyEmptyBuckets);
+};
+
+STATIC_ASSERT(std::is_standard_layout<PossiblyEmptyBuckets>::value);
+STATIC_ASSERT(sizeof(PossiblyEmptyBuckets) == kSystemPointerSize);
+
 // Data structure for maintaining a set of slots in a standard (non-large)
 // page.
 // The data structure assumes that the slots are pointer size aligned and
@@ -35,42 +134,33 @@ class SlotSet {
     KEEP_EMPTY_BUCKETS   // An empty bucket will be kept.
   };
 
-  enum class PossiblyEmpty : uint8_t {
-    kYes,       // Bucket is non-null but might be empty.
-    kNoOrNull,  // Bucket is null or cannot be empty.
-  };
-
   SlotSet() = delete;
 
   static SlotSet* Allocate(size_t buckets) {
-    //  SlotSet* slot_set ----------------------+
-    //                                          |
-    //                                          v
-    // +----------------------+-----------------+-------------------------+
-    // | possibly empty array | initial buckets |     buckets array       |
-    // +----------------------+-----------------+-------------------------+
-    //    1 byte * buckets       pointer-sized    pointer-sized * buckets
+    //  SlotSet* slot_set --+
+    //                      |
+    //                      v
+    //    +-----------------+-------------------------+
+    //    | initial buckets |     buckets array       |
+    //    +-----------------+-------------------------+
+    //       pointer-sized    pointer-sized * buckets
     //
     //
     // The SlotSet pointer points to the beginning of the buckets array for
     // faster access in the write barrier. The number of buckets is needed for
     // calculating the size of this data structure.
-    // Since pages can shrink we also store the initial_buckets size.
-    //
-    size_t possibly_empty_array_size = PossiblyEmptyArraySize(buckets);
     size_t buckets_size = buckets * sizeof(Bucket*);
-    size_t size =
-        possibly_empty_array_size + kInitialBucketsSize + buckets_size;
+    size_t size = kInitialBucketsSize + buckets_size;
     void* allocation = AlignedAlloc(size, kSystemPointerSize);
     SlotSet* slot_set = reinterpret_cast<SlotSet*>(
-        reinterpret_cast<uint8_t*>(allocation) + possibly_empty_array_size +
-        kInitialBucketsSize);
+        reinterpret_cast<uint8_t*>(allocation) + kInitialBucketsSize);
     DCHECK(
         IsAligned(reinterpret_cast<uintptr_t>(slot_set), kSystemPointerSize));
+#ifdef DEBUG
     *slot_set->initial_buckets() = buckets;
+#endif
     for (size_t i = 0; i < buckets; i++) {
       *slot_set->bucket(i) = nullptr;
-      *slot_set->possibly_empty(i) = PossiblyEmpty::kNoOrNull;
     }
     return slot_set;
   }
@@ -82,17 +172,15 @@ class SlotSet {
       slot_set->ReleaseBucket(i);
     }
 
+#ifdef DEBUG
     size_t initial_buckets = *slot_set->initial_buckets();
 
-#ifdef DEBUG
     for (size_t i = buckets; i < initial_buckets; i++) {
       DCHECK_NULL(*slot_set->bucket(i));
     }
 #endif
 
-    size_t possibly_empty_array_size = PossiblyEmptyArraySize(initial_buckets);
-    AlignedFree(reinterpret_cast<uint8_t*>(slot_set) - kInitialBucketsSize -
-                possibly_empty_array_size);
+    AlignedFree(reinterpret_cast<uint8_t*>(slot_set) - kInitialBucketsSize);
   }
 
   static size_t BucketsForSize(size_t size) {
@@ -260,13 +348,12 @@ class SlotSet {
   // Assumes that the possibly empty-array was already cleared by
   // CheckPossiblyEmptyBuckets.
   template <typename Callback>
-  size_t IterateAndTrackEmptyBuckets(Address chunk_start, size_t buckets,
-                                     Callback callback,
-                                     bool* empty_bucket_found) {
+  size_t IterateAndTrackEmptyBuckets(
+      Address chunk_start, size_t buckets, Callback callback,
+      PossiblyEmptyBuckets* possibly_empty_buckets) {
     return Iterate(chunk_start, buckets, callback,
-                   [this, empty_bucket_found](size_t bucket_index) {
-                     *possibly_empty(bucket_index) = PossiblyEmpty::kYes;
-                     *empty_bucket_found = true;
+                   [possibly_empty_buckets, buckets](size_t bucket_index) {
+                     possibly_empty_buckets->Insert(bucket_index, buckets);
                    });
   }
 
@@ -283,40 +370,29 @@ class SlotSet {
 
   // Check whether possibly empty buckets are really empty. Empty buckets are
   // freed and the possibly empty state is cleared for all buckets.
-  bool CheckPossiblyEmptyBuckets(size_t buckets) {
+  bool CheckPossiblyEmptyBuckets(size_t buckets,
+                                 PossiblyEmptyBuckets* possibly_empty_buckets) {
     bool empty = true;
     for (size_t bucket_index = 0; bucket_index < buckets; bucket_index++) {
       Bucket* bucket = LoadBucket<AccessMode::NON_ATOMIC>(bucket_index);
       if (bucket) {
-        if (*possibly_empty(bucket_index) == PossiblyEmpty::kYes) {
+        if (possibly_empty_buckets->Contains(bucket_index)) {
           if (bucket->IsEmpty()) {
             ReleaseBucket<AccessMode::NON_ATOMIC>(bucket_index);
           } else {
             empty = false;
           }
-          *possibly_empty(bucket_index) = PossiblyEmpty::kNoOrNull;
         } else {
           empty = false;
         }
       } else {
-        DCHECK_EQ(*possibly_empty(bucket_index), PossiblyEmpty::kNoOrNull);
+        DCHECK(!possibly_empty_buckets->Contains(bucket_index));
       }
     }
 
-    return empty;
-  }
-
-  // Check wether all possibly empty entries are cleared. Only used
-  // for testing in debug-builds.
-  bool IsPossiblyEmptyCleared() {
-    size_t buckets = *initial_buckets();
-    for (size_t bucket_index = 0; bucket_index < buckets; bucket_index++) {
-      if (*possibly_empty(bucket_index) != PossiblyEmpty::kNoOrNull) {
-        return false;
-      }
-    }
+    possibly_empty_buckets->Release();
 
-    return true;
+    return empty;
   }
 
   static const int kCellsPerBucket = 32;
@@ -500,20 +576,15 @@ class SlotSet {
     *bit_index = static_cast<int>(slot & (kBitsPerCell - 1));
   }
 
-  static size_t PossiblyEmptyArraySize(size_t buckets) {
-    return (sizeof(PossiblyEmpty) * buckets + (kSystemPointerSize - 1)) /
-           kSystemPointerSize * kSystemPointerSize;
-  }
-
   Bucket** buckets() { return reinterpret_cast<Bucket**>(this); }
   Bucket** bucket(size_t bucket_index) { return buckets() + bucket_index; }
-  PossiblyEmpty* possibly_empty(size_t bucket_index) {
-    return reinterpret_cast<PossiblyEmpty*>(buckets()) - kInitialBucketsSize -
-           1 - bucket_index;
-  }
 
+#ifdef DEBUG
   size_t* initial_buckets() { return reinterpret_cast<size_t*>(this) - 1; }
   static const int kInitialBucketsSize = sizeof(size_t);
+#else
+  static const int kInitialBucketsSize = 0;
+#endif
 };
 
 STATIC_ASSERT(std::is_standard_layout<SlotSet>::value);

src/heap/spaces.cc

@@ -750,6 +750,8 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
     chunk->code_object_registry_ = nullptr;
   }
 
+  chunk->possibly_empty_buckets_.Initialize();
+
   return chunk;
 }
 
@@ -1394,6 +1396,7 @@ void MemoryChunk::ReleaseAllocatedMemoryNeededForWritableChunk() {
     code_object_registry_ = nullptr;
   }
 
+  possibly_empty_buckets_.Release();
   ReleaseSlotSet<OLD_TO_NEW>();
   ReleaseSweepingSlotSet();
   ReleaseSlotSet<OLD_TO_OLD>();

src/heap/spaces.h

@@ -24,6 +24,7 @@
 #include "src/heap/heap.h"
 #include "src/heap/invalidated-slots.h"
 #include "src/heap/marking.h"
+#include "src/heap/slot-set.h"
 #include "src/objects/free-space.h"
 #include "src/objects/heap-object.h"
 #include "src/objects/map.h"
@@ -622,7 +623,8 @@ class MemoryChunk : public BasicMemoryChunk {
       + kSystemPointerSize      // LocalArrayBufferTracker* local_tracker_
       + kIntptrSize  // std::atomic<intptr_t> young_generation_live_byte_count_
       + kSystemPointerSize   // Bitmap* young_generation_bitmap_
-      + kSystemPointerSize;  // CodeObjectRegistry* code_object_registry_
+      + kSystemPointerSize   // CodeObjectRegistry* code_object_registry_
+      + kSystemPointerSize;  // PossiblyEmptyBuckets possibly_empty_buckets_
 
   // Page size in bytes.  This must be a multiple of the OS page size.
   static const int kPageSize = 1 << kPageSizeBits;
@@ -869,6 +871,10 @@ class MemoryChunk : public BasicMemoryChunk {
 
   FreeList* free_list() { return owner()->free_list(); }
 
+  PossiblyEmptyBuckets* possibly_empty_buckets() {
+    return &possibly_empty_buckets_;
+  }
+
  protected:
   static MemoryChunk* Initialize(Heap* heap, Address base, size_t size,
                                  Address area_start, Address area_end,
@@ -964,6 +970,8 @@ class MemoryChunk : public BasicMemoryChunk {
 
   CodeObjectRegistry* code_object_registry_;
 
+  PossiblyEmptyBuckets possibly_empty_buckets_;
+
  private:
   void InitializeReservedMemory() { reservation_.Reset(); }
 

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

@@ -110,6 +110,20 @@ TEST(SlotSet, Remove) {
   SlotSet::Delete(set, SlotSet::kBucketsRegularPage);
 }
 
+TEST(PossiblyEmptyBuckets, ContainsAndInsert) {
+  static const int kBuckets = 100;
+  PossiblyEmptyBuckets possibly_empty_buckets;
+  possibly_empty_buckets.Insert(0, kBuckets);
+  int last = sizeof(uintptr_t) * kBitsPerByte - 2;
+  possibly_empty_buckets.Insert(last, kBuckets);
+  EXPECT_TRUE(possibly_empty_buckets.Contains(0));
+  EXPECT_TRUE(possibly_empty_buckets.Contains(last));
+  possibly_empty_buckets.Insert(last + 1, kBuckets);
+  EXPECT_TRUE(possibly_empty_buckets.Contains(0));
+  EXPECT_TRUE(possibly_empty_buckets.Contains(last));
+  EXPECT_TRUE(possibly_empty_buckets.Contains(last + 1));
+}
+
 void CheckRemoveRangeOn(uint32_t start, uint32_t end) {
   SlotSet* set = SlotSet::Allocate(SlotSet::kBucketsRegularPage);
   uint32_t first = start == 0 ? 0 : start - kTaggedSize;