Revert of [heap] Reland Concurrently free empty typed slot set chunks....

Revert of [heap] Reland Concurrently free empty typed slot set chunks. (patchset #2 id:20001 of https://codereview.chromium.org/2365603002/ )

Reason for revert:
Leaks and TSAN:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20ASAN/builds/15441
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN/builds/11867

Original issue's description:
> [heap] Reland Concurrently free empty typed slot set chunks.
>
> BUG=chromium:648568
>
> Committed: https://crrev.com/ec3835751d678c0b4a3fda47d11ffa107cd27e23
> Cr-Commit-Position: refs/heads/master@{#39630}

TBR=ulan@chromium.org,hpayer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:648568

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

src/heap/mark-compact.cc

@@ -3865,12 +3865,6 @@ int MarkCompactCollector::Sweeper::ParallelSweepPage(Page* page,
     } else {
       max_freed = RawSweep(page, REBUILD_FREE_LIST, free_space_mode);
     }
-
-    // After finishing sweeping of a page we clean up its remembered set.
-    if (page->typed_old_to_new_slots()) {
-      page->typed_old_to_new_slots()->FreeToBeFreedChunks();
-    }
-
     {
       base::LockGuard<base::Mutex> guard(&mutex_);
       swept_list_[identity].Add(page);

src/heap/remembered-set.cc

@@ -36,8 +36,7 @@ void RememberedSet<direction>::ClearInvalidSlots(Heap* heap) {
             } else {
               return REMOVE_SLOT;
             }
-          },
-          TypedSlotSet::PREFREE_EMPTY_CHUNKS);
+          });
     }
   }
   for (MemoryChunk* chunk : *heap->map_space()) {

src/heap/remembered-set.h

@@ -149,13 +149,10 @@ class RememberedSet {
   static void RemoveRangeTyped(MemoryChunk* page, Address start, Address end) {
     TypedSlotSet* slots = GetTypedSlotSet(page);
     if (slots != nullptr) {
-      slots->Iterate(
-          [start, end](SlotType slot_type, Address host_addr,
-                       Address slot_addr) {
-            return start <= slot_addr && slot_addr < end ? REMOVE_SLOT
-                                                         : KEEP_SLOT;
-          },
-          TypedSlotSet::PREFREE_EMPTY_CHUNKS);
+      slots->Iterate([start, end](SlotType slot_type, Address host_addr,
+                                  Address slot_addr) {
+        return start <= slot_addr && slot_addr < end ? REMOVE_SLOT : KEEP_SLOT;
+      });
     }
   }
 
@@ -176,7 +173,7 @@ class RememberedSet {
   static void IterateTyped(MemoryChunk* chunk, Callback callback) {
     TypedSlotSet* slots = GetTypedSlotSet(chunk);
     if (slots != nullptr) {
-      int new_count = slots->Iterate(callback, TypedSlotSet::KEEP_EMPTY_CHUNKS);
+      int new_count = slots->Iterate(callback);
       if (new_count == 0) {
         ReleaseTypedSlotSet(chunk);
       }

src/heap/slot-set.h

@@ -5,8 +5,6 @@
 #ifndef V8_SLOT_SET_H
 #define V8_SLOT_SET_H
 
-#include <stack>
-
 #include "src/allocation.h"
 #include "src/base/atomic-utils.h"
 #include "src/base/bits.h"
@@ -263,8 +261,6 @@ enum SlotType {
 // typed slots contain V8 internal pointers that are not directly exposed to JS.
 class TypedSlotSet {
  public:
-  enum IterationMode { PREFREE_EMPTY_CHUNKS, KEEP_EMPTY_CHUNKS };
-
   typedef std::pair<SlotType, uint32_t> TypeAndOffset;
 
   struct TypedSlot {
@@ -332,10 +328,6 @@ class TypedSlotSet {
   void Insert(SlotType type, uint32_t host_offset, uint32_t offset) {
     TypedSlot slot(type, host_offset, offset);
     Chunk* top_chunk = chunk_.Value();
-    if (!top_chunk) {
-      top_chunk = new Chunk(nullptr, kInitialBufferSize);
-      chunk_.SetValue(top_chunk);
-    }
     if (!top_chunk->AddSlot(slot)) {
       Chunk* new_top_chunk =
           new Chunk(top_chunk, NextCapacity(top_chunk->capacity.Value()));
@@ -356,15 +348,13 @@ class TypedSlotSet {
   //    else return REMOVE_SLOT;
   // });
   template <typename Callback>
-  int Iterate(Callback callback, IterationMode mode) {
+  int Iterate(Callback callback) {
     STATIC_ASSERT(CLEARED_SLOT < 8);
     Chunk* chunk = chunk_.Value();
-    Chunk* previous = nullptr;
     int new_count = 0;
     while (chunk != nullptr) {
       TypedSlot* buffer = chunk->buffer.Value();
       int count = chunk->count.Value();
-      bool empty = true;
       for (int i = 0; i < count; i++) {
         // Order is important here. We have to read out the slot type last to
         // observe the concurrent removal case consistently.
@@ -375,40 +365,16 @@ class TypedSlotSet {
           Address addr = page_start_ + type_and_offset.second;
           if (callback(type, host_addr, addr) == KEEP_SLOT) {
             new_count++;
-            empty = false;
           } else {
             buffer[i].Clear();
           }
         }
       }
-
-      if (mode == PREFREE_EMPTY_CHUNKS && empty) {
-        // We remove the chunk from the list but let it still point its next
-        // chunk to allow concurrent iteration.
-        if (previous) {
-          previous->next.SetValue(chunk->next.Value());
-        } else {
-          chunk_.SetValue(chunk->next.Value());
-        }
-        base::LockGuard<base::Mutex> guard(&to_be_freed_chunks_mutex_);
-        to_be_freed_chunks_.push(chunk);
-      } else {
-        previous = chunk;
-      }
       chunk = chunk->next.Value();
     }
     return new_count;
   }
 
-  void FreeToBeFreedChunks() {
-    base::LockGuard<base::Mutex> guard(&to_be_freed_chunks_mutex_);
-    while (!to_be_freed_chunks_.empty()) {
-      Chunk* top = to_be_freed_chunks_.top();
-      to_be_freed_chunks_.pop();
-      delete top;
-    }
-  }
-
  private:
   static const int kInitialBufferSize = 100;
   static const int kMaxBufferSize = 16 * KB;
@@ -447,8 +413,6 @@ class TypedSlotSet {
 
   Address page_start_;
   base::AtomicValue<Chunk*> chunk_;
-  base::Mutex to_be_freed_chunks_mutex_;
-  std::stack<Chunk*> to_be_freed_chunks_;
 };
 
 }  // namespace internal

src/heap/spaces.cc

@@ -510,7 +510,7 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
   chunk->InitializeReservedMemory();
   chunk->old_to_new_slots_ = nullptr;
   chunk->old_to_old_slots_ = nullptr;
-  chunk->typed_old_to_new_slots_.SetValue(nullptr);
+  chunk->typed_old_to_new_slots_ = nullptr;
   chunk->typed_old_to_old_slots_ = nullptr;
   chunk->skip_list_ = nullptr;
   chunk->write_barrier_counter_ = kWriteBarrierCounterGranularity;
@@ -1077,7 +1077,7 @@ void MemoryChunk::ReleaseAllocatedMemory() {
   }
   if (old_to_new_slots_ != nullptr) ReleaseOldToNewSlots();
   if (old_to_old_slots_ != nullptr) ReleaseOldToOldSlots();
-  if (typed_old_to_new_slots_.Value() != nullptr) ReleaseTypedOldToNewSlots();
+  if (typed_old_to_new_slots_ != nullptr) ReleaseTypedOldToNewSlots();
   if (typed_old_to_old_slots_ != nullptr) ReleaseTypedOldToOldSlots();
   if (local_tracker_ != nullptr) ReleaseLocalTracker();
 }
@@ -1113,14 +1113,13 @@ void MemoryChunk::ReleaseOldToOldSlots() {
 }
 
 void MemoryChunk::AllocateTypedOldToNewSlots() {
-  DCHECK(nullptr == typed_old_to_new_slots_.Value());
-  typed_old_to_new_slots_.SetValue(new TypedSlotSet(address()));
+  DCHECK(nullptr == typed_old_to_new_slots_);
+  typed_old_to_new_slots_ = new TypedSlotSet(address());
 }
 
 void MemoryChunk::ReleaseTypedOldToNewSlots() {
-  TypedSlotSet* typed_old_to_new_slots = typed_old_to_new_slots_.Value();
-  delete typed_old_to_new_slots;
-  typed_old_to_new_slots_.SetValue(nullptr);
+  delete typed_old_to_new_slots_;
+  typed_old_to_new_slots_ = nullptr;
 }
 
 void MemoryChunk::AllocateTypedOldToOldSlots() {

src/heap/spaces.h

@@ -456,7 +456,7 @@ class MemoryChunk {
   inline SlotSet* old_to_new_slots() { return old_to_new_slots_; }
   inline SlotSet* old_to_old_slots() { return old_to_old_slots_; }
   inline TypedSlotSet* typed_old_to_new_slots() {
-    return typed_old_to_new_slots_.Value();
+    return typed_old_to_new_slots_;
   }
   inline TypedSlotSet* typed_old_to_old_slots() {
     return typed_old_to_old_slots_;
@@ -656,7 +656,7 @@ class MemoryChunk {
   // is ceil(size() / kPageSize).
   SlotSet* old_to_new_slots_;
   SlotSet* old_to_old_slots_;
-  base::AtomicValue<TypedSlotSet*> typed_old_to_new_slots_;
+  TypedSlotSet* typed_old_to_new_slots_;
   TypedSlotSet* typed_old_to_old_slots_;
 
   SkipList* skip_list_;

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

@@ -152,29 +152,24 @@ TEST(TypedSlotSet, Iterate) {
     ++added;
   }
   int iterated = 0;
-  set.Iterate(
-      [&iterated, kDelta, kHostDelta](SlotType type, Address host_addr,
-                                      Address addr) {
-        uint32_t i = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(addr));
-        uint32_t j =
-            static_cast<uint32_t>(reinterpret_cast<uintptr_t>(host_addr));
-        EXPECT_EQ(i % CLEARED_SLOT, static_cast<uint32_t>(type));
-        EXPECT_EQ(0, i % kDelta);
-        EXPECT_EQ(0, j % kHostDelta);
-        ++iterated;
-        return i % 2 == 0 ? KEEP_SLOT : REMOVE_SLOT;
-      },
-      TypedSlotSet::KEEP_EMPTY_CHUNKS);
+  set.Iterate([&iterated, kDelta, kHostDelta](SlotType type, Address host_addr,
+                                              Address addr) {
+    uint32_t i = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(addr));
+    uint32_t j = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(host_addr));
+    EXPECT_EQ(i % CLEARED_SLOT, static_cast<uint32_t>(type));
+    EXPECT_EQ(0, i % kDelta);
+    EXPECT_EQ(0, j % kHostDelta);
+    ++iterated;
+    return i % 2 == 0 ? KEEP_SLOT : REMOVE_SLOT;
+  });
   EXPECT_EQ(added, iterated);
   iterated = 0;
-  set.Iterate(
-      [&iterated](SlotType type, Address host_addr, Address addr) {
-        uint32_t i = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(addr));
-        EXPECT_EQ(0, i % 2);
-        ++iterated;
-        return KEEP_SLOT;
-      },
-      TypedSlotSet::KEEP_EMPTY_CHUNKS);
+  set.Iterate([&iterated](SlotType type, Address host_addr, Address addr) {
+    uint32_t i = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(addr));
+    EXPECT_EQ(0, i % 2);
+    ++iterated;
+    return KEEP_SLOT;
+  });
   EXPECT_EQ(added / 2, iterated);
 }