[zone-stats] Cleanup ZoneList interface

... and introduce a bottleneck for collecting reusable zone memory
statistics.

Tbr: jgruber@chromium.org
Bug: v8:10572
Change-Id: I418f8b495c0d89c0eb73f4e19bc4315acfadb480
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2287500Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68776}

src/ast/scopes.cc

@@ -1514,7 +1514,7 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   DCHECK(is_function_scope());
 
   // Reset all non-trivial members.
-  params_.Clear();
+  params_.DropAndClear();
   decls_.Clear();
   locals_.Clear();
   inner_scope_ = nullptr;

src/regexp/regexp-compiler-tonode.cc

@@ -391,7 +391,8 @@ void AddUnicodeCaseEquivalents(ZoneList<CharacterRange>* ranges, Zone* zone) {
   for (int i = 0; i < ranges->length(); i++) {
     set.add(ranges->at(i).from(), ranges->at(i).to());
   }
-  ranges->Clear();
+  // Clear the ranges list without freeing the backing store.
+  ranges->Rewind(0);
   set.closeOver(USET_CASE_INSENSITIVE);
   // Full case mapping map single characters to multiple characters.
   // Those are represented as strings in the set. Remove them so that

src/zone/zone-list-inl.h

@@ -18,7 +18,7 @@ void ZoneList<T>::Add(const T& element, Zone* zone) {
   if (length_ < capacity_) {
     data_[length_++] = element;
   } else {
-    ZoneList<T>::ResizeAdd(element, ZoneAllocationPolicy(zone));
+    ZoneList<T>::ResizeAdd(element, zone);
   }
 }
 
@@ -30,12 +30,11 @@ void ZoneList<T>::AddAll(const ZoneList<T>& other, Zone* zone) {
 template <typename T>
 void ZoneList<T>::AddAll(const Vector<T>& other, Zone* zone) {
   int result_length = length_ + other.length();
-  if (capacity_ < result_length)
-    Resize(result_length, ZoneAllocationPolicy(zone));
+  if (capacity_ < result_length) Resize(result_length, zone);
   if (std::is_fundamental<T>()) {
-    memcpy(data_ + length_, other.begin(), sizeof(*data_) * other.length());
+    memcpy(&data_[length_], other.begin(), sizeof(T) * other.length());
   } else {
-    for (int i = 0; i < other.length(); i++) data_[length_ + i] = other.at(i);
+    std::copy(other.begin(), other.end(), &data_[length_]);
   }
   length_ = result_length;
 }
@@ -43,13 +42,12 @@ void ZoneList<T>::AddAll(const Vector<T>& other, Zone* zone) {
 // Use two layers of inlining so that the non-inlined function can
 // use the same implementation as the inlined version.
 template <typename T>
-void ZoneList<T>::ResizeAdd(const T& element, ZoneAllocationPolicy alloc) {
-  ResizeAddInternal(element, alloc);
+void ZoneList<T>::ResizeAdd(const T& element, Zone* zone) {
+  ResizeAddInternal(element, zone);
 }
 
 template <typename T>
-void ZoneList<T>::ResizeAddInternal(const T& element,
-                                    ZoneAllocationPolicy alloc) {
+void ZoneList<T>::ResizeAddInternal(const T& element, Zone* zone) {
   DCHECK(length_ >= capacity_);
   // Grow the list capacity by 100%, but make sure to let it grow
   // even when the capacity is zero (possible initial case).
@@ -57,18 +55,22 @@ void ZoneList<T>::ResizeAddInternal(const T& element,
   // Since the element reference could be an element of the list, copy
   // it out of the old backing storage before resizing.
   T temp = element;
-  Resize(new_capacity, alloc);
+  Resize(new_capacity, zone);
   data_[length_++] = temp;
 }
 
 template <typename T>
-void ZoneList<T>::Resize(int new_capacity, ZoneAllocationPolicy alloc) {
+void ZoneList<T>::Resize(int new_capacity, Zone* zone) {
   DCHECK_LE(length_, new_capacity);
-  T* new_data = NewData(new_capacity, alloc);
+  T* new_data = zone->NewArray<T>(new_capacity);
   if (length_ > 0) {
-    MemCopy(new_data, data_, length_ * sizeof(T));
+    if (std::is_fundamental<T>()) {
+      MemCopy(new_data, data_, length_ * sizeof(T));
+    } else {
+      std::copy(&data_[0], &data_[length_], &new_data[0]);
+    }
   }
-  ZoneList<T>::DeleteData(data_);
+  if (data_) zone->DeleteArray<T>(data_, capacity_);
   data_ = new_data;
   capacity_ = new_capacity;
 }
@@ -108,13 +110,9 @@ T ZoneList<T>::Remove(int i) {
 }
 
 template <typename T>
-void ZoneList<T>::Clear() {
-  DeleteData(data_);
-  // We don't call Initialize(0) since that requires passing a Zone,
-  // which we don't really need.
-  data_ = nullptr;
-  capacity_ = 0;
-  length_ = 0;
+void ZoneList<T>::Clear(Zone* zone) {
+  if (data_) zone->DeleteArray<T>(data_, capacity_);
+  DropAndClear();
 }
 
 template <typename T>

src/zone/zone-list.h

@@ -17,35 +17,32 @@ namespace internal {
 template <typename T>
 class Vector;
 
-// ZoneLists are growable lists with constant-time access to the
-// elements. The list itself and all its elements are allocated in the
-// Zone. ZoneLists cannot be deleted individually; you can delete all
-// objects in the Zone by calling Zone::DeleteAll().
+// ZoneLists are growable lists with constant-time access to the elements.
+// The list itself and all its elements are supposed to be allocated in zone
+// memory. Unlike ZoneVector container, the ZoneList instance has minimal
+// possible size which makes it a good candidate for embedding into other
+// often-allocated zone objects.
+//
+// Note, ZoneLists' elements cannot be deleted individually and the destructor
+// intentionally does not free the backing store. Because of the latter, the
+// ZoneList must not be used outsize of zone memory. Consider using ZoneVector
+// or other containers instead.
 template <typename T>
-class ZoneList final {
+class ZoneList final : public ZoneObject {
  public:
   // Construct a new ZoneList with the given capacity; the length is
   // always zero. The capacity must be non-negative.
   ZoneList(int capacity, Zone* zone) { Initialize(capacity, zone); }
-  // Construct a new ZoneList from a std::initializer_list
-  ZoneList(std::initializer_list<T> list, Zone* zone) {
-    Initialize(static_cast<int>(list.size()), zone);
-    for (auto& i : list) Add(i, zone);
-  }
   // Construct a new ZoneList by copying the elements of the given ZoneList.
   ZoneList(const ZoneList<T>& other, Zone* zone) {
     Initialize(other.length(), zone);
     AddAll(other, zone);
   }
 
-  V8_INLINE ~ZoneList() { DeleteData(data_); }
-
-  // Please the MSVC compiler.  We should never have to execute this.
-  V8_INLINE void operator delete(void* p, ZoneAllocationPolicy allocator) {
-    UNREACHABLE();
-  }
-
-  void* operator new(size_t size, Zone* zone) { return zone->New(size); }
+  // The ZoneList objects are usually allocated as a fields in other
+  // zone-allocated objects for which destructors are not called anyway, so
+  // we are not going to clear the memory here as well.
+  ~ZoneList() = default;
 
   // Returns a reference to the element at index i. This reference is not safe
   // to use after operations that can change the list's backing store
@@ -69,18 +66,21 @@ class ZoneList final {
 
   Vector<T> ToVector() const { return Vector<T>(data_, length_); }
   Vector<T> ToVector(int start, int length) const {
-    return Vector<T>(data_ + start, std::min(length_ - start, length));
+    DCHECK_LE(start, length_);
+    return Vector<T>(&data_[start], std::min(length_ - start, length));
   }
 
   Vector<const T> ToConstVector() const {
     return Vector<const T>(data_, length_);
   }
 
+  // TODO(v8:10572): remove this method in favor of more flexible constructor,
+  // allowing initialization from provided iterators.
   V8_INLINE void Initialize(int capacity, Zone* zone) {
     DCHECK_GE(capacity, 0);
-    data_ = (capacity > 0) ? NewData(capacity, ZoneAllocationPolicy(zone))
-                           : nullptr;
+    DCHECK_NULL(data_);
     capacity_ = capacity;
+    data_ = (capacity_ > 0) ? zone->NewArray<T>(capacity_) : nullptr;
     length_ = 0;
   }
 
@@ -115,7 +115,16 @@ class ZoneList final {
   // Clears the list by freeing the storage memory. If you want to keep the
   // memory, use Rewind(0) instead. Be aware, that even if T is a
   // pointer type, clearing the list doesn't delete the entries.
-  V8_INLINE void Clear();
+  V8_INLINE void Clear(Zone* zone);
+
+  // Clears the list but unlike Clear(), it doesn't free the storage memory.
+  // It's useful when the whole zone containing the backing store will be
+  // released but the list will be used further.
+  V8_INLINE void DropAndClear() {
+    data_ = nullptr;
+    capacity_ = 0;
+    length_ = 0;
+  }
 
   // Drops all but the first 'pos' elements from the list.
   V8_INLINE void Rewind(int pos);
@@ -137,29 +146,21 @@ class ZoneList final {
   template <typename CompareFunction>
   void StableSort(CompareFunction cmp, size_t start, size_t length);
 
-  void operator delete(void* pointer) { UNREACHABLE(); }
-  void operator delete(void* pointer, Zone* zone) { UNREACHABLE(); }
-
  private:
-  T* data_;
-  int capacity_;
-  int length_;
-
-  V8_INLINE T* NewData(int n, ZoneAllocationPolicy allocator) {
-    return static_cast<T*>(allocator.New(n * sizeof(T)));
-  }
-  V8_INLINE void DeleteData(T* data) { ZoneAllocationPolicy::Delete(data); }
+  T* data_ = nullptr;
+  int capacity_ = 0;
+  int length_ = 0;
 
   // Increase the capacity of a full list, and add an element.
   // List must be full already.
-  void ResizeAdd(const T& element, ZoneAllocationPolicy allocator);
+  void ResizeAdd(const T& element, Zone* zone);
 
   // Inlined implementation of ResizeAdd, shared by inlined and
   // non-inlined versions of ResizeAdd.
-  void ResizeAddInternal(const T& element, ZoneAllocationPolicy allocator);
+  void ResizeAddInternal(const T& element, Zone* zone);
 
   // Resize the list.
-  void Resize(int new_capacity, ZoneAllocationPolicy allocator);
+  void Resize(int new_capacity, Zone* zone);
 
   DISALLOW_COPY_AND_ASSIGN(ZoneList);
 };

src/zone/zone.h

@@ -63,6 +63,18 @@ class V8_EXPORT_PRIVATE Zone final {
     return static_cast<T*>(New(length * sizeof(T)));
   }
 
+  template <typename T>
+  void DeleteArray(T* pointer, size_t length) {
+    DCHECK_NOT_NULL(pointer);
+    DCHECK_NE(length, 0);
+    // TODO(v8:10572): implement accounting for reusable zone memory
+#ifdef DEBUG
+    size_t size = RoundUp(length * sizeof(T), kAlignmentInBytes);
+    static const unsigned char kZapDeadByte = 0xcd;
+    memset(pointer, kZapDeadByte, size);
+#endif
+  }
+
   // Seals the zone to prevent any further allocation.
   void Seal() { sealed_ = true; }