Add RecyclingZoneAllocator for ZoneDeque.

A std::deque interacts badly with zone memory in that it allocates chunks
of memory for the back of the queue and frees memory from the front of the
queue. As such we never reuse zone memory for the queue. Implement a very
simple RecyclingZoneAllocator which keeps a single block of memory from
deallocation that can be reused on allocation.

Also clean up zone-allocator a bit and make it use proper Chromium coding
style.

BUG=chromium:700364

Change-Id: I19330a8a9ec6d75fe18d8168d41f1a12030a6c4d
Reviewed-on: https://chromium-review.googlesource.com/458916Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44154}

src/compiler/instruction.h

@@ -1469,7 +1469,8 @@ std::ostream& operator<<(std::ostream& os,
 
 typedef ZoneDeque<Constant> ConstantDeque;
 typedef std::map<int, Constant, std::less<int>,
-                 zone_allocator<std::pair<const int, Constant> > > ConstantMap;
+                 ZoneAllocator<std::pair<const int, Constant> > >
+    ConstantMap;
 
 typedef ZoneDeque<Instruction*> InstructionDeque;
 typedef ZoneDeque<ReferenceMap*> ReferenceMapDeque;

src/objects.cc

@@ -19686,38 +19686,38 @@ struct StringHandleEqual {
 class UnorderedStringSet
     : public std::unordered_set<Handle<String>, StringHandleHash,
                                 StringHandleEqual,
-                                zone_allocator<Handle<String>>> {
+                                ZoneAllocator<Handle<String>>> {
  public:
   explicit UnorderedStringSet(Zone* zone)
       : std::unordered_set<Handle<String>, StringHandleHash, StringHandleEqual,
-                           zone_allocator<Handle<String>>>(
+                           ZoneAllocator<Handle<String>>>(
             2 /* bucket count */, StringHandleHash(), StringHandleEqual(),
-            zone_allocator<Handle<String>>(zone)) {}
+            ZoneAllocator<Handle<String>>(zone)) {}
 };
 
 class UnorderedModuleSet
     : public std::unordered_set<Handle<Module>, ModuleHandleHash,
                                 ModuleHandleEqual,
-                                zone_allocator<Handle<Module>>> {
+                                ZoneAllocator<Handle<Module>>> {
  public:
   explicit UnorderedModuleSet(Zone* zone)
       : std::unordered_set<Handle<Module>, ModuleHandleHash, ModuleHandleEqual,
-                           zone_allocator<Handle<Module>>>(
+                           ZoneAllocator<Handle<Module>>>(
             2 /* bucket count */, ModuleHandleHash(), ModuleHandleEqual(),
-            zone_allocator<Handle<Module>>(zone)) {}
+            ZoneAllocator<Handle<Module>>(zone)) {}
 };
 
 class UnorderedStringMap
     : public std::unordered_map<
           Handle<String>, Handle<Object>, StringHandleHash, StringHandleEqual,
-          zone_allocator<std::pair<const Handle<String>, Handle<Object>>>> {
+          ZoneAllocator<std::pair<const Handle<String>, Handle<Object>>>> {
  public:
   explicit UnorderedStringMap(Zone* zone)
       : std::unordered_map<
             Handle<String>, Handle<Object>, StringHandleHash, StringHandleEqual,
-            zone_allocator<std::pair<const Handle<String>, Handle<Object>>>>(
+            ZoneAllocator<std::pair<const Handle<String>, Handle<Object>>>>(
             2 /* bucket count */, StringHandleHash(), StringHandleEqual(),
-            zone_allocator<std::pair<const Handle<String>, Handle<Object>>>(
+            ZoneAllocator<std::pair<const Handle<String>, Handle<Object>>>(
                 zone)) {}
 };
 
@@ -19726,17 +19726,17 @@ class UnorderedStringMap
 class Module::ResolveSet
     : public std::unordered_map<
           Handle<Module>, UnorderedStringSet*, ModuleHandleHash,
-          ModuleHandleEqual, zone_allocator<std::pair<const Handle<Module>,
-                                                      UnorderedStringSet*>>> {
+          ModuleHandleEqual,
+          ZoneAllocator<std::pair<const Handle<Module>, UnorderedStringSet*>>> {
  public:
   explicit ResolveSet(Zone* zone)
       : std::unordered_map<Handle<Module>, UnorderedStringSet*,
                            ModuleHandleHash, ModuleHandleEqual,
-                           zone_allocator<std::pair<const Handle<Module>,
-                                                    UnorderedStringSet*>>>(
+                           ZoneAllocator<std::pair<const Handle<Module>,
+                                                   UnorderedStringSet*>>>(
             2 /* bucket count */, ModuleHandleHash(), ModuleHandleEqual(),
-            zone_allocator<
-                std::pair<const Handle<Module>, UnorderedStringSet*>>(zone)),
+            ZoneAllocator<std::pair<const Handle<Module>, UnorderedStringSet*>>(
+                zone)),
         zone_(zone) {}
 
   Zone* zone() const { return zone_; }

src/zone/zone-allocator.h

@@ -12,7 +12,7 @@ namespace v8 {
 namespace internal {
 
 template <typename T>
-class zone_allocator {
+class ZoneAllocator {
  public:
   typedef T* pointer;
   typedef const T* const_pointer;
@@ -23,33 +23,33 @@ class zone_allocator {
   typedef ptrdiff_t difference_type;
   template <class O>
   struct rebind {
-    typedef zone_allocator<O> other;
+    typedef ZoneAllocator<O> other;
   };
 
 #ifdef V8_CC_MSVC
   // MSVS unfortunately requires the default constructor to be defined.
-  zone_allocator() : zone_(nullptr) { UNREACHABLE(); }
+  ZoneAllocator() : ZoneAllocator(nullptr) { UNREACHABLE(); }
 #endif
-  explicit zone_allocator(Zone* zone) throw() : zone_(zone) {}
-  explicit zone_allocator(const zone_allocator& other) throw()
-      : zone_(other.zone_) {}
+  explicit ZoneAllocator(Zone* zone) throw() : zone_(zone) {}
+  explicit ZoneAllocator(const ZoneAllocator& other) throw()
+      : ZoneAllocator<T>(other.zone_) {}
   template <typename U>
-  zone_allocator(const zone_allocator<U>& other) throw() : zone_(other.zone_) {}
+  ZoneAllocator(const ZoneAllocator<U>& other) throw()
+      : ZoneAllocator<T>(other.zone_) {}
   template <typename U>
-  friend class zone_allocator;
+  friend class ZoneAllocator;
 
-  pointer address(reference x) const { return &x; }
-  const_pointer address(const_reference x) const { return &x; }
+  T* address(T& x) const { return &x; }
+  const T* address(const T& x) const { return &x; }
 
-  pointer allocate(size_type n, const void* hint = 0) {
-    return static_cast<pointer>(
-        zone_->NewArray<value_type>(static_cast<int>(n)));
+  T* allocate(size_t n, const void* hint = 0) {
+    return static_cast<T*>(zone_->NewArray<T>(static_cast<int>(n)));
   }
-  void deallocate(pointer p, size_type) { /* noop for Zones */
+  void deallocate(T* p, size_t) { /* noop for Zones */
   }
 
-  size_type max_size() const throw() {
-    return std::numeric_limits<int>::max() / sizeof(value_type);
+  size_t max_size() const throw() {
+    return std::numeric_limits<int>::max() / sizeof(T);
   }
   template <typename U, typename... Args>
   void construct(U* p, Args&&... args) {
@@ -61,10 +61,10 @@ class zone_allocator {
     p->~U();
   }
 
-  bool operator==(zone_allocator const& other) const {
+  bool operator==(ZoneAllocator const& other) const {
     return zone_ == other.zone_;
   }
-  bool operator!=(zone_allocator const& other) const {
+  bool operator!=(ZoneAllocator const& other) const {
     return zone_ != other.zone_;
   }
 
@@ -74,8 +74,75 @@ class zone_allocator {
   Zone* zone_;
 };
 
-typedef zone_allocator<bool> ZoneBoolAllocator;
-typedef zone_allocator<int> ZoneIntAllocator;
+// A recycling zone allocator maintains a free list of deallocated chunks
+// to reuse on subsiquent allocations. The free list management is purposely
+// very simple and works best for data-structures which regularly allocate and
+// free blocks of similar sized memory (such as std::deque).
+template <typename T>
+class RecyclingZoneAllocator : public ZoneAllocator<T> {
+ public:
+  template <class O>
+  struct rebind {
+    typedef RecyclingZoneAllocator<O> other;
+  };
+
+#ifdef V8_CC_MSVC
+  // MSVS unfortunately requires the default constructor to be defined.
+  RecyclingZoneAllocator()
+      : ZoneAllocator(nullptr, nullptr), free_list_(nullptr) {
+    UNREACHABLE();
+  }
+#endif
+  explicit RecyclingZoneAllocator(Zone* zone) throw()
+      : ZoneAllocator<T>(zone), free_list_(nullptr) {}
+  explicit RecyclingZoneAllocator(const RecyclingZoneAllocator& other) throw()
+      : ZoneAllocator<T>(other), free_list_(nullptr) {}
+  template <typename U>
+  RecyclingZoneAllocator(const RecyclingZoneAllocator<U>& other) throw()
+      : ZoneAllocator<T>(other), free_list_(nullptr) {}
+  template <typename U>
+  friend class RecyclingZoneAllocator;
+
+  T* allocate(size_t n, const void* hint = 0) {
+    // Only check top block in free list, since this will be equal to or larger
+    // than the other blocks in the free list.
+    if (free_list_ && free_list_->size >= n) {
+      T* return_val = reinterpret_cast<T*>(free_list_);
+      free_list_ = free_list_->next;
+      return return_val;
+    } else {
+      return ZoneAllocator<T>::allocate(n, hint);
+    }
+  }
+
+  void deallocate(T* p, size_t n) {
+    if ((sizeof(T) * n < sizeof(FreeBlock))) return;
+
+    // Only add block to free_list if it is equal or larger than previous block
+    // so that allocation stays O(1) only having to look at the top block.
+    if (!free_list_ || free_list_->size <= n) {
+      // Store the free-list within the block being deallocated.
+      DCHECK((sizeof(T) * n >= sizeof(FreeBlock)));
+      FreeBlock* new_free_block = reinterpret_cast<FreeBlock*>(p);
+
+      new_free_block->size = n;
+      new_free_block->next = free_list_;
+      free_list_ = new_free_block;
+    }
+  }
+
+ private:
+  struct FreeBlock {
+    FreeBlock* next;
+    size_t size;
+  };
+
+  FreeBlock* free_list_;
+};
+
+typedef ZoneAllocator<bool> ZoneBoolAllocator;
+typedef ZoneAllocator<int> ZoneIntAllocator;
+
 }  // namespace internal
 }  // namespace v8
 

src/zone/zone-containers.h

@@ -21,38 +21,38 @@ namespace internal {
 // A wrapper subclass for std::vector to make it easy to construct one
 // that uses a zone allocator.
 template <typename T>
-class ZoneVector : public std::vector<T, zone_allocator<T>> {
+class ZoneVector : public std::vector<T, ZoneAllocator<T>> {
  public:
   // Constructs an empty vector.
   explicit ZoneVector(Zone* zone)
-      : std::vector<T, zone_allocator<T>>(zone_allocator<T>(zone)) {}
+      : std::vector<T, ZoneAllocator<T>>(ZoneAllocator<T>(zone)) {}
 
   // Constructs a new vector and fills it with {size} elements, each
   // constructed via the default constructor.
   ZoneVector(size_t size, Zone* zone)
-      : std::vector<T, zone_allocator<T>>(size, T(), zone_allocator<T>(zone)) {}
+      : std::vector<T, ZoneAllocator<T>>(size, T(), ZoneAllocator<T>(zone)) {}
 
   // Constructs a new vector and fills it with {size} elements, each
   // having the value {def}.
   ZoneVector(size_t size, T def, Zone* zone)
-      : std::vector<T, zone_allocator<T>>(size, def, zone_allocator<T>(zone)) {}
+      : std::vector<T, ZoneAllocator<T>>(size, def, ZoneAllocator<T>(zone)) {}
 
   // Constructs a new vector and fills it with the contents of the range
   // [first, last).
   template <class InputIt>
   ZoneVector(InputIt first, InputIt last, Zone* zone)
-      : std::vector<T, zone_allocator<T>>(first, last,
-                                          zone_allocator<T>(zone)) {}
+      : std::vector<T, ZoneAllocator<T>>(first, last, ZoneAllocator<T>(zone)) {}
 };
 
 // A wrapper subclass std::deque to make it easy to construct one
 // that uses a zone allocator.
 template <typename T>
-class ZoneDeque : public std::deque<T, zone_allocator<T>> {
+class ZoneDeque : public std::deque<T, RecyclingZoneAllocator<T>> {
  public:
   // Constructs an empty deque.
   explicit ZoneDeque(Zone* zone)
-      : std::deque<T, zone_allocator<T>>(zone_allocator<T>(zone)) {}
+      : std::deque<T, RecyclingZoneAllocator<T>>(
+            RecyclingZoneAllocator<T>(zone)) {}
 };
 
 // A wrapper subclass std::list to make it easy to construct one
@@ -60,11 +60,11 @@ class ZoneDeque : public std::deque<T, zone_allocator<T>> {
 // TODO(mstarzinger): This should be renamed to ZoneList once we got rid of our
 // own home-grown ZoneList that actually is a ZoneVector.
 template <typename T>
-class ZoneLinkedList : public std::list<T, zone_allocator<T>> {
+class ZoneLinkedList : public std::list<T, ZoneAllocator<T>> {
  public:
   // Constructs an empty list.
   explicit ZoneLinkedList(Zone* zone)
-      : std::list<T, zone_allocator<T>>(zone_allocator<T>(zone)) {}
+      : std::list<T, ZoneAllocator<T>>(ZoneAllocator<T>(zone)) {}
 };
 
 // A wrapper subclass std::priority_queue to make it easy to construct one
@@ -102,24 +102,24 @@ class ZoneStack : public std::stack<T, ZoneDeque<T>> {
 // A wrapper subclass for std::set to make it easy to construct one that uses
 // a zone allocator.
 template <typename K, typename Compare = std::less<K>>
-class ZoneSet : public std::set<K, Compare, zone_allocator<K>> {
+class ZoneSet : public std::set<K, Compare, ZoneAllocator<K>> {
  public:
   // Constructs an empty set.
   explicit ZoneSet(Zone* zone)
-      : std::set<K, Compare, zone_allocator<K>>(Compare(),
-                                                zone_allocator<K>(zone)) {}
+      : std::set<K, Compare, ZoneAllocator<K>>(Compare(),
+                                               ZoneAllocator<K>(zone)) {}
 };
 
 // A wrapper subclass for std::map to make it easy to construct one that uses
 // a zone allocator.
 template <typename K, typename V, typename Compare = std::less<K>>
 class ZoneMap
-    : public std::map<K, V, Compare, zone_allocator<std::pair<const K, V>>> {
+    : public std::map<K, V, Compare, ZoneAllocator<std::pair<const K, V>>> {
  public:
   // Constructs an empty map.
   explicit ZoneMap(Zone* zone)
-      : std::map<K, V, Compare, zone_allocator<std::pair<const K, V>>>(
-            Compare(), zone_allocator<std::pair<const K, V>>(zone)) {}
+      : std::map<K, V, Compare, ZoneAllocator<std::pair<const K, V>>>(
+            Compare(), ZoneAllocator<std::pair<const K, V>>(zone)) {}
 };
 
 // A wrapper subclass for std::multimap to make it easy to construct one that
@@ -127,12 +127,12 @@ class ZoneMap
 template <typename K, typename V, typename Compare = std::less<K>>
 class ZoneMultimap
     : public std::multimap<K, V, Compare,
-                           zone_allocator<std::pair<const K, V>>> {
+                           ZoneAllocator<std::pair<const K, V>>> {
  public:
   // Constructs an empty multimap.
   explicit ZoneMultimap(Zone* zone)
-      : std::multimap<K, V, Compare, zone_allocator<std::pair<const K, V>>>(
-            Compare(), zone_allocator<std::pair<const K, V>>(zone)) {}
+      : std::multimap<K, V, Compare, ZoneAllocator<std::pair<const K, V>>>(
+            Compare(), ZoneAllocator<std::pair<const K, V>>(zone)) {}
 };
 
 // Typedefs to shorten commonly used vectors.

test/unittests/BUILD.gn

@@ -149,6 +149,7 @@ v8_executable("unittests") {
     "wasm/wasm-module-builder-unittest.cc",
     "wasm/wasm-opcodes-unittest.cc",
     "zone/segmentpool-unittest.cc",
+    "zone/zone-allocator-unittest.cc",
     "zone/zone-chunk-list-unittest.cc",
     "zone/zone-unittest.cc",
   ]

test/unittests/unittests.gyp

@@ -136,6 +136,7 @@
       'unicode-unittest.cc',
       'value-serializer-unittest.cc',
       'zone/segmentpool-unittest.cc',
+      'zone/zone-allocator-unittest.cc',
       'zone/zone-chunk-list-unittest.cc',
       'zone/zone-unittest.cc',
       'wasm/asm-types-unittest.cc',

test/unittests/zone/zone-allocator-unittest.cc

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/zone/zone-allocator.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace internal {
+
+TEST(RecyclingZoneAllocator, ReuseSameSize) {
+  AccountingAllocator accounting_allocator;
+  Zone zone(&accounting_allocator, ZONE_NAME);
+  RecyclingZoneAllocator<int> zone_allocator(&zone);
+
+  int* allocated = zone_allocator.allocate(10);
+  zone_allocator.deallocate(allocated, 10);
+  CHECK_EQ(zone_allocator.allocate(10), allocated);
+}
+
+TEST(RecyclingZoneAllocator, ReuseSmallerSize) {
+  AccountingAllocator accounting_allocator;
+  Zone zone(&accounting_allocator, ZONE_NAME);
+  RecyclingZoneAllocator<int> zone_allocator(&zone);
+
+  int* allocated = zone_allocator.allocate(100);
+  zone_allocator.deallocate(allocated, 100);
+  CHECK_EQ(zone_allocator.allocate(10), allocated);
+}
+
+TEST(RecyclingZoneAllocator, DontReuseTooSmallSize) {
+  AccountingAllocator accounting_allocator;
+  Zone zone(&accounting_allocator, ZONE_NAME);
+  RecyclingZoneAllocator<int> zone_allocator(&zone);
+
+  // The sizeof(FreeBlock) will be larger than a single int, so we can't keep
+  // store the free list in the deallocated block.
+  int* allocated = zone_allocator.allocate(1);
+  zone_allocator.deallocate(allocated, 1);
+  CHECK_NE(zone_allocator.allocate(1), allocated);
+}
+
+TEST(RecyclingZoneAllocator, ReuseMultipleSize) {
+  AccountingAllocator accounting_allocator;
+  Zone zone(&accounting_allocator, ZONE_NAME);
+  RecyclingZoneAllocator<int> zone_allocator(&zone);
+
+  int* allocated1 = zone_allocator.allocate(10);
+  int* allocated2 = zone_allocator.allocate(20);
+  int* allocated3 = zone_allocator.allocate(30);
+  zone_allocator.deallocate(allocated1, 10);
+  zone_allocator.deallocate(allocated2, 20);
+  zone_allocator.deallocate(allocated3, 30);
+  CHECK_EQ(zone_allocator.allocate(10), allocated3);
+  CHECK_EQ(zone_allocator.allocate(10), allocated2);
+  CHECK_EQ(zone_allocator.allocate(10), allocated1);
+}
+
+TEST(RecyclingZoneAllocator, DontChainSmallerSizes) {
+  AccountingAllocator accounting_allocator;
+  Zone zone(&accounting_allocator, ZONE_NAME);
+  RecyclingZoneAllocator<int> zone_allocator(&zone);
+
+  int* allocated1 = zone_allocator.allocate(10);
+  int* allocated2 = zone_allocator.allocate(5);
+  int* allocated3 = zone_allocator.allocate(10);
+  zone_allocator.deallocate(allocated1, 10);
+  zone_allocator.deallocate(allocated2, 5);
+  zone_allocator.deallocate(allocated3, 10);
+  CHECK_EQ(zone_allocator.allocate(5), allocated3);
+  CHECK_EQ(zone_allocator.allocate(5), allocated1);
+  CHECK_NE(zone_allocator.allocate(5), allocated2);
+}
+
+}  // namespace internal
+}  // namespace v8