Revert of Pool implementation for zone segments (patchset #13 id:520001 of...

Revert of Pool implementation for zone segments (patchset #13 id:520001 of https://codereview.chromium.org/2335343007/ )

Reason for revert:
blocks roll: https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/294283/steps/compile%20%28with%20patch%29/logs/stdio

Original issue's description:
> Pool implementation for zone segments
>
> BUG=v8:5409
>
> Committed: https://crrev.com/37c688a24578e787d3d8941093563ed049c3497e
> Committed: https://crrev.com/316669f62ea3834395bf4caab7bc3d7c32f6bbc6
> Cr-Original-Commit-Position: refs/heads/master@{#39631}
> Cr-Commit-Position: refs/heads/master@{#40044}

TBR=jochen@chromium.org,jkummerow@chromium.org,verwaest@chromium.org,yangguo@chromium.org,heimbuef@google.com
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=v8:5409

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

src/api.cc

@@ -8291,7 +8291,6 @@ void Isolate::IsolateInBackgroundNotification() {
 void Isolate::MemoryPressureNotification(MemoryPressureLevel level) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
   isolate->heap()->MemoryPressureNotification(level, Locker::IsLocked(this));
-  isolate->allocator()->MemoryPressureNotification(level);
 }
 
 void Isolate::SetRAILMode(RAILMode rail_mode) {

src/isolate.cc

@@ -1934,8 +1934,8 @@ class VerboseAccountingAllocator : public AccountingAllocator {
   VerboseAccountingAllocator(Heap* heap, size_t sample_bytes)
       : heap_(heap), last_memory_usage_(0), sample_bytes_(sample_bytes) {}
 
-  v8::internal::Segment* GetSegment(size_t size) override {
-    v8::internal::Segment* memory = AccountingAllocator::GetSegment(size);
+  v8::internal::Segment* AllocateSegment(size_t size) override {
+    v8::internal::Segment* memory = AccountingAllocator::AllocateSegment(size);
     if (memory) {
       size_t current = GetCurrentMemoryUsage();
       if (last_memory_usage_.Value() + sample_bytes_ < current) {
@@ -1946,8 +1946,8 @@ class VerboseAccountingAllocator : public AccountingAllocator {
     return memory;
   }
 
-  void ReturnSegment(v8::internal::Segment* memory) override {
-    AccountingAllocator::ReturnSegment(memory);
+  void FreeSegment(v8::internal::Segment* memory) override {
+    AccountingAllocator::FreeSegment(memory);
     size_t current = GetCurrentMemoryUsage();
     if (current + sample_bytes_ < last_memory_usage_.Value()) {
       PrintJSON(current);

src/runtime/runtime-regexp.cc

@@ -381,7 +381,7 @@ void FindStringIndicesDispatch(Isolate* isolate, String* subject,
 }
 
 namespace {
-List<int>* GetRewoundRegexpIndicesList(Isolate* isolate) {
+List<int>* GetRewindedRegexpIndicesList(Isolate* isolate) {
   List<int>* list = isolate->regexp_indices();
   list->Rewind(0);
   return list;
@@ -404,7 +404,7 @@ MUST_USE_RESULT static Object* StringReplaceGlobalAtomRegExpWithString(
   DCHECK(subject->IsFlat());
   DCHECK(replacement->IsFlat());
 
-  List<int>* indices = GetRewoundRegexpIndicesList(isolate);
+  List<int>* indices = GetRewindedRegexpIndicesList(isolate);
 
   DCHECK_EQ(JSRegExp::ATOM, pattern_regexp->TypeTag());
   String* pattern =
@@ -721,7 +721,7 @@ RUNTIME_FUNCTION(Runtime_StringSplit) {
   subject = String::Flatten(subject);
   pattern = String::Flatten(pattern);
 
-  List<int>* indices = GetRewoundRegexpIndicesList(isolate);
+  List<int>* indices = GetRewindedRegexpIndicesList(isolate);
 
   FindStringIndicesDispatch(isolate, *subject, *pattern, indices, limit);
 

src/v8.gyp

@@ -1272,8 +1272,6 @@
         'zone/zone-segment.h',
         'zone/zone.cc',
         'zone/zone.h',
-        'zone/zone-segment.cc',
-        'zone/zone-segment.h',
         'zone/zone-allocator.h',
         'zone/zone-containers.h',
       ],

src/zone/accounting-allocator.cc

@@ -13,39 +13,6 @@
 namespace v8 {
 namespace internal {
 
-AccountingAllocator::AccountingAllocator() : unused_segments_mutex_() {
-  memory_pressure_level_.SetValue(MemoryPressureLevel::kNone);
-  std::fill(unused_segments_heads_,
-            unused_segments_heads_ +
-                (1 + kMaxSegmentSizePower - kMinSegmentSizePower),
-            nullptr);
-  std::fill(
-      unused_segments_sizes,
-      unused_segments_sizes + (1 + kMaxSegmentSizePower - kMinSegmentSizePower),
-      0);
-}
-
-AccountingAllocator::~AccountingAllocator() { ClearPool(); }
-
-void AccountingAllocator::MemoryPressureNotification(
-    MemoryPressureLevel level) {
-  memory_pressure_level_.SetValue(level);
-
-  if (level != MemoryPressureLevel::kNone) {
-    ClearPool();
-  }
-}
-
-Segment* AccountingAllocator::GetSegment(size_t bytes) {
-  Segment* result = GetSegmentFromPool(bytes);
-  if (result == nullptr) {
-    result = AllocateSegment(bytes);
-    result->Initialize(bytes);
-  }
-
-  return result;
-}
-
 Segment* AccountingAllocator::AllocateSegment(size_t bytes) {
   void* memory = malloc(bytes);
   if (memory) {
@@ -59,16 +26,6 @@ Segment* AccountingAllocator::AllocateSegment(size_t bytes) {
   return reinterpret_cast<Segment*>(memory);
 }
 
-void AccountingAllocator::ReturnSegment(Segment* segment) {
-  segment->ZapContents();
-
-  if (memory_pressure_level_.Value() != MemoryPressureLevel::kNone) {
-    FreeSegment(segment);
-  } else if (!AddSegmentToPool(segment)) {
-    FreeSegment(segment);
-  }
-}
-
 void AccountingAllocator::FreeSegment(Segment* memory) {
   base::NoBarrier_AtomicIncrement(
       &current_memory_usage_, -static_cast<base::AtomicWord>(memory->size()));
@@ -84,82 +41,5 @@ size_t AccountingAllocator::GetMaxMemoryUsage() const {
   return base::NoBarrier_Load(&max_memory_usage_);
 }
 
-Segment* AccountingAllocator::GetSegmentFromPool(size_t requested_size) {
-  if (requested_size > (1 << kMaxSegmentSizePower)) {
-    return nullptr;
-  }
-
-  uint8_t power = kMinSegmentSizePower;
-  while (requested_size > static_cast<size_t>(1 << power)) power++;
-
-  DCHECK_GE(power, kMinSegmentSizePower + 0);
-  power -= kMinSegmentSizePower;
-
-  Segment* segment;
-  {
-    base::LockGuard<base::Mutex> lock_guard(&unused_segments_mutex_);
-
-    segment = unused_segments_heads_[power];
-
-    if (segment != nullptr) {
-      unused_segments_heads_[power] = segment->next();
-      segment->set_next(nullptr);
-
-      unused_segments_sizes[power]--;
-      unused_segments_size_ -= segment->size();
-    }
-  }
-
-  if (segment) {
-    DCHECK_GE(segment->size(), requested_size);
-  }
-  return segment;
-}
-
-bool AccountingAllocator::AddSegmentToPool(Segment* segment) {
-  size_t size = segment->size();
-
-  if (size >= (1 << (kMaxSegmentSizePower + 1))) return false;
-
-  if (size < (1 << kMinSegmentSizePower)) return false;
-
-  uint8_t power = kMaxSegmentSizePower;
-
-  while (size < static_cast<size_t>(1 << power)) power--;
-
-  DCHECK_GE(power, kMinSegmentSizePower + 0);
-  power -= kMinSegmentSizePower;
-
-  {
-    base::LockGuard<base::Mutex> lock_guard(&unused_segments_mutex_);
-
-    if (unused_segments_sizes[power] >= kMaxSegmentsPerBucket) {
-      return false;
-    }
-
-    segment->set_next(unused_segments_heads_[power]);
-    unused_segments_heads_[power] = segment;
-    unused_segments_size_ += size;
-    unused_segments_sizes[power]++;
-  }
-
-  return true;
-}
-
-void AccountingAllocator::ClearPool() {
-  base::LockGuard<base::Mutex> lock_guard(&unused_segments_mutex_);
-
-  for (uint8_t power = 0; power <= kMaxSegmentSizePower - kMinSegmentSizePower;
-       power++) {
-    Segment* current = unused_segments_heads_[power];
-    while (current) {
-      Segment* next = current->next();
-      FreeSegment(current);
-      current = next;
-    }
-    unused_segments_heads_[power] = nullptr;
-  }
-}
-
 }  // namespace internal
 }  // namespace v8

src/zone/accounting-allocator.h

@@ -19,55 +19,19 @@ namespace internal {
 
 class V8_EXPORT_PRIVATE AccountingAllocator {
  public:
-  AccountingAllocator();
-  virtual ~AccountingAllocator();
+  AccountingAllocator() = default;
+  virtual ~AccountingAllocator() = default;
 
-  // Gets an empty segment from the pool or creates a new one.
-  virtual Segment* GetSegment(size_t bytes);
-  // Return unneeded segments to either insert them into the pool or release
-  // them if the pool is already full or memory pressure is high.
-  virtual void ReturnSegment(Segment* memory);
+  virtual Segment* AllocateSegment(size_t bytes);
+  virtual void FreeSegment(Segment* memory);
 
   size_t GetCurrentMemoryUsage() const;
   size_t GetMaxMemoryUsage() const;
 
-  size_t GetCurrentPoolSize() const;
-
-  void MemoryPressureNotification(MemoryPressureLevel level);
-
  private:
-  static const uint8_t kMinSegmentSizePower = 13;
-  static const uint8_t kMaxSegmentSizePower = 18;
-  static const uint8_t kMaxSegmentsPerBucket = 5;
-
-  STATIC_ASSERT(kMinSegmentSizePower <= kMaxSegmentSizePower);
-
-  // Allocates a new segment. Returns nullptr on failed allocation.
-  Segment* AllocateSegment(size_t bytes);
-  void FreeSegment(Segment* memory);
-
-  // Returns a segment from the pool of at least the requested size.
-  Segment* GetSegmentFromPool(size_t requested_size);
-  // Trys to add a segment to the pool. Returns false if the pool is full.
-  bool AddSegmentToPool(Segment* segment);
-
-  // Empties the pool and puts all its contents onto the garbage stack.
-  void ClearPool();
-
-  Segment*
-      unused_segments_heads_[1 + kMaxSegmentSizePower - kMinSegmentSizePower];
-
-  size_t unused_segments_sizes[1 + kMaxSegmentSizePower - kMinSegmentSizePower];
-
-  size_t unused_segments_size_ = 0;
-
-  base::Mutex unused_segments_mutex_;
-
   base::AtomicWord current_memory_usage_ = 0;
   base::AtomicWord max_memory_usage_ = 0;
 
-  base::AtomicValue<MemoryPressureLevel> memory_pressure_level_;
-
   DISALLOW_COPY_AND_ASSIGN(AccountingAllocator);
 };
 

src/zone/zone-segment.cc

@@ -18,6 +18,5 @@ void Segment::ZapHeader() {
   memset(this, kZapDeadByte, sizeof(Segment));
 #endif
 }
-
 }  // namespace internal
 }  // namespace v8

src/zone/zone-segment.h

@@ -20,7 +20,11 @@ class Zone;
 
 class Segment {
  public:
-  void Initialize(size_t size) { size_ = size; }
+  void Initialize(Segment* next, size_t size, Zone* zone) {
+    next_ = next;
+    size_ = size;
+    zone_ = zone;
+  }
 
   Zone* zone() const { return zone_; }
   void set_zone(Zone* const zone) { zone_ = zone; }
@@ -44,7 +48,6 @@ class Segment {
   // Constant byte value used for zapping dead memory in debug mode.
   static const unsigned char kZapDeadByte = 0xcd;
 #endif
-
   // Computes the address of the nth byte in this segment.
   Address address(size_t n) const { return Address(this) + n; }
 

src/zone/zone.cc

@@ -51,6 +51,7 @@ Zone::Zone(AccountingAllocator* allocator)
 
 Zone::~Zone() {
   DeleteAll();
+  DeleteKeptSegment();
 
   DCHECK(segment_bytes_allocated_ == 0);
 }
@@ -91,35 +92,73 @@ void* Zone::New(size_t size) {
 }
 
 void Zone::DeleteAll() {
-  // Traverse the chained list of segments and return them all to the allocator.
+  // Find a segment with a suitable size to keep around.
+  Segment* keep = nullptr;
+  // Traverse the chained list of segments, zapping (in debug mode)
+  // and freeing every segment except the one we wish to keep.
   for (Segment* current = segment_head_; current;) {
     Segment* next = current->next();
-    size_t size = current->size();
-
-    // Un-poison the segment content so we can re-use or zap it later.
-    ASAN_UNPOISON_MEMORY_REGION(current->start(), current->capacity());
-
-    segment_bytes_allocated_ -= size;
-    allocator_->ReturnSegment(current);
+    if (!keep && current->size() <= kMaximumKeptSegmentSize) {
+      // Unlink the segment we wish to keep from the list.
+      keep = current;
+      keep->set_next(nullptr);
+    } else {
+      size_t size = current->size();
+#ifdef DEBUG
+      // Un-poison first so the zapping doesn't trigger ASan complaints.
+      ASAN_UNPOISON_MEMORY_REGION(current, size);
+#endif
+      current->ZapContents();
+      segment_bytes_allocated_ -= size;
+      allocator_->FreeSegment(current);
+    }
     current = next;
   }
 
-  position_ = limit_ = 0;
+  // If we have found a segment we want to keep, we must recompute the
+  // variables 'position' and 'limit' to prepare for future allocate
+  // attempts. Otherwise, we must clear the position and limit to
+  // force a new segment to be allocated on demand.
+  if (keep) {
+    Address start = keep->start();
+    position_ = RoundUp(start, kAlignment);
+    limit_ = keep->end();
+    // Un-poison so we can re-use the segment later.
+    ASAN_UNPOISON_MEMORY_REGION(start, keep->capacity());
+    keep->ZapContents();
+  } else {
+    position_ = limit_ = 0;
+  }
 
   allocation_size_ = 0;
   // Update the head segment to be the kept segment (if any).
-  segment_head_ = nullptr;
+  segment_head_ = keep;
+}
+
+void Zone::DeleteKeptSegment() {
+  DCHECK(segment_head_ == nullptr || segment_head_->next() == nullptr);
+  if (segment_head_ != nullptr) {
+    size_t size = segment_head_->size();
+#ifdef DEBUG
+    // Un-poison first so the zapping doesn't trigger ASan complaints.
+    ASAN_UNPOISON_MEMORY_REGION(segment_head_, size);
+#endif
+    segment_head_->ZapContents();
+    segment_bytes_allocated_ -= size;
+    allocator_->FreeSegment(segment_head_);
+    segment_head_ = nullptr;
+  }
+
+  DCHECK(segment_bytes_allocated_ == 0);
 }
 
 // Creates a new segment, sets it size, and pushes it to the front
 // of the segment chain. Returns the new segment.
-Segment* Zone::NewSegment(size_t requested_size) {
-  Segment* result = allocator_->GetSegment(requested_size);
-  DCHECK_GE(result->size(), requested_size);
-  segment_bytes_allocated_ += result->size();
+Segment* Zone::NewSegment(size_t size) {
+  Segment* result = allocator_->AllocateSegment(size);
+  segment_bytes_allocated_ += size;
   if (result != nullptr) {
-    result->set_zone(this);
-    result->set_next(segment_head_);
+    result->Initialize(segment_head_, size, this);
     segment_head_ = result;
   }
   return result;

src/zone/zone.h

@@ -25,7 +25,7 @@ namespace internal {
 //
 // Note: There is no need to initialize the Zone; the first time an
 // allocation is attempted, a segment of memory will be requested
-// through the allocator.
+// through a call to malloc().
 //
 // Note: The implementation is inherently not thread safe. Do not use
 // from multi-threaded code.
@@ -44,9 +44,14 @@ class V8_EXPORT_PRIVATE Zone final {
     return static_cast<T*>(New(length * sizeof(T)));
   }
 
-  // Deletes all objects and free all memory allocated in the Zone.
+  // Deletes all objects and free all memory allocated in the Zone. Keeps one
+  // small (size <= kMaximumKeptSegmentSize) segment around if it finds one.
   void DeleteAll();
 
+  // Deletes the last small segment kept around by DeleteAll(). You
+  // may no longer allocate in the Zone after a call to this method.
+  void DeleteKeptSegment();
+
   // Returns true if more memory has been allocated in zones than
   // the limit allows.
   bool excess_allocation() const {
@@ -75,6 +80,9 @@ class V8_EXPORT_PRIVATE Zone final {
   // Never allocate segments larger than this size in bytes.
   static const size_t kMaximumSegmentSize = 1 * MB;
 
+  // Never keep segments larger than this size in bytes around.
+  static const size_t kMaximumKeptSegmentSize = 64 * KB;
+
   // Report zone excess when allocation exceeds this limit.
   static const size_t kExcessLimit = 256 * MB;
 
@@ -94,7 +102,7 @@ class V8_EXPORT_PRIVATE Zone final {
 
   // Creates a new segment, sets it size, and pushes it to the front
   // of the segment chain. Returns the new segment.
-  inline Segment* NewSegment(size_t requested_size);
+  inline Segment* NewSegment(size_t size);
 
   // The free region in the current (front) segment is represented as
   // the half-open interval [position, limit). The 'position' variable