Bug 1474605 - Tidy up LifoAlloc a little r=nbp
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 11 Jul 2018 10:31:38 +0100
changeset 426193 b3468b999b15d5b91c8d55a9c3745e4d4f17591e
parent 426192 b765ddf11188c523ac7855bdebeb342439053dc0
child 426194 e378e29a5ebbbff1605efee6739cbf3d4581101f
push id34267
push userrgurzau@mozilla.com
push dateWed, 11 Jul 2018 22:05:21 +0000
treeherdermozilla-central@3aca103e4915 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1474605
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1474605 - Tidy up LifoAlloc a little r=nbp
js/src/ds/LifoAlloc.cpp
js/src/ds/LifoAlloc.h
js/src/jsutil.h
--- a/js/src/ds/LifoAlloc.cpp
+++ b/js/src/ds/LifoAlloc.cpp
@@ -139,42 +139,60 @@ BumpChunk::removeMProtectHandler() const
 }
 
 #endif
 
 } // namespace detail
 } // namespace js
 
 void
+LifoAlloc::reset(size_t defaultChunkSize)
+{
+    MOZ_ASSERT(mozilla::IsPowerOfTwo(defaultChunkSize));
+
+    while (!chunks_.empty()) {
+        chunks_.begin()->setRWUntil(Loc::End);
+        chunks_.popFirst();
+    }
+    while (!unused_.empty()) {
+        unused_.begin()->setRWUntil(Loc::End);
+        unused_.popFirst();
+    }
+    defaultChunkSize_ = defaultChunkSize;
+    markCount = 0;
+    curSize_ = 0;
+}
+
+void
 LifoAlloc::freeAll()
 {
     while (!chunks_.empty()) {
         chunks_.begin()->setRWUntil(Loc::End);
-        BumpChunk bc = chunks_.popFirst();
+        UniqueBumpChunk bc = chunks_.popFirst();
         decrementCurSize(bc->computedSizeOfIncludingThis());
     }
     while (!unused_.empty()) {
         unused_.begin()->setRWUntil(Loc::End);
-        BumpChunk bc = unused_.popFirst();
+        UniqueBumpChunk bc = unused_.popFirst();
         decrementCurSize(bc->computedSizeOfIncludingThis());
     }
 
     // Nb: maintaining curSize_ correctly isn't easy.  Fortunately, this is an
     // excellent sanity check.
     MOZ_ASSERT(curSize_ == 0);
 }
 
-LifoAlloc::BumpChunk
+LifoAlloc::UniqueBumpChunk
 LifoAlloc::newChunkWithCapacity(size_t n)
 {
     MOZ_ASSERT(fallibleScope_, "[OOM] Cannot allocate a new chunk in an infallible scope.");
 
     // Compute the size which should be requested in order to be able to fit |n|
     // bytes in the newly allocated chunk, or default the |defaultChunkSize_|.
-    size_t defaultChunkFreeSpace = defaultChunkSize_ - detail::BumpChunk::reservedSpace;
+    size_t defaultChunkFreeSpace = defaultChunkSize_ - detail::BumpChunkReservedSpace;
     size_t chunkSize;
     if (n > defaultChunkFreeSpace) {
         MOZ_ASSERT(defaultChunkFreeSpace < defaultChunkSize_);
         size_t allocSizeWithCanaries = n + (defaultChunkSize_ - defaultChunkFreeSpace);
 
         // Guard for overflow.
         if (allocSizeWithCanaries < n ||
             (allocSizeWithCanaries & (size_t(1) << (BitSize<size_t>::value - 1))))
@@ -188,17 +206,17 @@ LifoAlloc::newChunkWithCapacity(size_t n
     }
 
     bool protect = false;
 #ifdef LIFO_CHUNK_PROTECT
     protect = protect_;
 #endif
 
     // Create a new BumpChunk, and allocate space for it.
-    BumpChunk result = detail::BumpChunk::newWithCapacity(chunkSize, protect);
+    UniqueBumpChunk result = detail::BumpChunk::newWithCapacity(chunkSize, protect);
     if (!result)
         return nullptr;
     MOZ_ASSERT(result->computedSizeOfIncludingThis() == chunkSize);
     return result;
 }
 
 bool
 LifoAlloc::getOrCreateChunk(size_t n)
@@ -233,17 +251,17 @@ LifoAlloc::getOrCreateChunk(size_t n)
                 unused_.appendAll(std::move(temp));
                 chunks_.last()->setRWUntil(Loc::End);
                 return true;
             }
         }
     }
 
     // Allocate a new BumpChunk with enough space for the next allocation.
-    BumpChunk newChunk = newChunkWithCapacity(n);
+    UniqueBumpChunk newChunk = newChunkWithCapacity(n);
     if (!newChunk)
         return false;
     size_t size = newChunk->computedSizeOfIncludingThis();
     // The last chunk in which allocations are performed should be protected
     // with setRWUntil(Loc::End), but this is not necessary here because any new
     // allocation should be protected as RW already.
     protectLast();
     chunks_.append(std::move(newChunk));
--- a/js/src/ds/LifoAlloc.h
+++ b/js/src/ds/LifoAlloc.h
@@ -26,46 +26,46 @@
 #include "jsutil.h"
 
 #include "js/UniquePtr.h"
 
 namespace js {
 
 namespace detail {
 
-template <typename T>
+template <typename T, typename D>
 class SingleLinkedList;
 
-template <typename T>
+template <typename T, typename D = JS::DeletePolicy<T>>
 class SingleLinkedListElement
 {
-    friend class SingleLinkedList<T>;
-    js::UniquePtr<T> next_;
+    friend class SingleLinkedList<T, D>;
+    js::UniquePtr<T, D> next_;
 
   public:
     SingleLinkedListElement()
       : next_(nullptr)
     {}
     ~SingleLinkedListElement() {
         MOZ_ASSERT(!next_);
     }
 
     T* next() const { return next_.get(); }
 };
 
 // Single linked list which is using UniquePtr to hold the next pointers.
 // UniquePtr are used to ensure that none of the elements are used
 // silmutaneously in 2 different list.
-template <typename T>
+template <typename T, typename D = JS::DeletePolicy<T>>
 class SingleLinkedList
 {
   private:
     // First element of the list which owns the next element, and ensure that
     // that this list is the only owner of the element.
-    UniquePtr<T> head_;
+    UniquePtr<T, D> head_;
 
     // Weak pointer to the last element of the list.
     T* last_;
 
     void assertInvariants() {
         MOZ_ASSERT(bool(head_) == bool(last_));
         MOZ_ASSERT_IF(last_, !last_->next_);
     }
@@ -146,25 +146,25 @@ class SingleLinkedList
             result.last_ = last_;
             last_ = newLast;
         }
         assertInvariants();
         result.assertInvariants();
         return result;
     }
 
-    void pushFront(UniquePtr<T>&& elem) {
+    void pushFront(UniquePtr<T, D>&& elem) {
         if (!last_)
             last_ = elem.get();
         elem->next_ = std::move(head_);
         head_ = std::move(elem);
         assertInvariants();
     }
 
-    void append(UniquePtr<T>&& elem) {
+    void append(UniquePtr<T, D>&& elem) {
         if (last_) {
             last_->next_ = std::move(elem);
             last_ = last_->next_.get();
         } else {
             head_ = std::move(elem);
             last_ = head_.get();
         }
         assertInvariants();
@@ -176,34 +176,33 @@ class SingleLinkedList
             last_->next_ = std::move(list.head_);
         else
             head_ = std::move(list.head_);
         last_ = list.last_;
         list.last_ = nullptr;
         assertInvariants();
         list.assertInvariants();
     }
-    UniquePtr<T> popFirst() {
+    UniquePtr<T, D> popFirst() {
         MOZ_ASSERT(head_);
-        UniquePtr<T> result = std::move(head_);
+        UniquePtr<T, D> result = std::move(head_);
         head_ = std::move(result->next_);
         if (!head_)
             last_ = nullptr;
         assertInvariants();
         return result;
     }
 };
 
 static const size_t LIFO_ALLOC_ALIGN = 8;
 
 MOZ_ALWAYS_INLINE
 uint8_t*
 AlignPtr(uint8_t* orig) {
-    static_assert(mozilla::tl::FloorLog2<LIFO_ALLOC_ALIGN>::value ==
-                  mozilla::tl::CeilingLog2<LIFO_ALLOC_ALIGN>::value,
+    static_assert(mozilla::IsPowerOfTwo(LIFO_ALLOC_ALIGN),
                   "LIFO_ALLOC_ALIGN must be a power of two");
 
     uint8_t* result = (uint8_t*) AlignBytes(uintptr_t(orig), LIFO_ALLOC_ALIGN);
     MOZ_ASSERT(uintptr_t(result) % LIFO_ALLOC_ALIGN == 0);
     return result;
 }
 
 // A Chunk represent a single memory allocation made with the system
@@ -284,25 +283,16 @@ class BumpChunk : public SingleLinkedLis
         capacity_(base() + capacity)
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
       , magic_(magicNumber)
 #endif
 #ifdef LIFO_CHUNK_PROTECT
       , protect_(protect ? 1 : 0)
 #endif
     {
-        // We cannot bake this value inside the BumpChunk class, because
-        // sizeof(BumpChunk) can only be computed after the closing brace of the
-        // BumpChunk class, or within one of its methods. As a work-around, the
-        // reservedSpace value is baked in, and we check that it indeed matches
-        // with the space taken by the data of the BumpChunk class, and the
-        // alignment of a pointer.
-        MOZ_ASSERT(BumpChunk::reservedSpace == AlignBytes(sizeof(BumpChunk), LIFO_ALLOC_ALIGN),
-                   "Checked that the baked-in value correspond to computed value");
-
         assertInvariants();
 #if defined(LIFO_HAVE_MEM_CHECKS)
         // The memory is freshly allocated and marked as undefined by the
         // allocator of the BumpChunk. Instead, we mark this memory as
         // no-access, as it has not been allocated within the BumpChunk.
         LIFO_MAKE_MEM_NOACCESS(bump_, capacity_ - bump_);
 #endif
         addMProtectHandler();
@@ -338,33 +328,28 @@ class BumpChunk : public SingleLinkedLis
     }
 
   public:
     ~BumpChunk() {
         release();
         removeMProtectHandler();
     }
 
-    // Space reserved for the BumpChunk internal data, and the alignment of the
-    // first allocation content.  This can be used to ensure there is enough
-    // space for the next allocation. (see LifoAlloc::newChunkWithCapacity)
-    static constexpr size_t reservedSpace = 4 * sizeof(uintptr_t);
-
     // Returns true if this chunk contains no allocated content.
     bool empty() const { return end() == begin(); }
 
     // Returns the size in bytes of the number of allocated space. This includes
     // the size consumed by the alignment of the allocations.
     size_t used() const { return end() - begin(); }
 
     // These are used for manipulating a chunk as if it was a vector of bytes,
     // and used for iterating over the content of the buffer (see
     // LifoAlloc::Enum)
-    const uint8_t* begin() const { return base() + reservedSpace; }
-    uint8_t* begin() { return base() + reservedSpace; }
+    inline const uint8_t* begin() const;
+    inline uint8_t* begin();
     uint8_t* end() const { return bump_; }
 
     // This function is the only way to allocate and construct a chunk. It
     // returns a UniquePtr to the newly allocated chunk.  The size given as
     // argument includes the space needed for the header of the chunk.
     //
     // The protect boolean is used to indicate whether the Bumpchunk memory
     // should be reported within the MemoryProtectionExceptionHandler.
@@ -465,30 +450,30 @@ class BumpChunk : public SingleLinkedLis
     // These locations are approximated locations, with the base rounded up to
     // the nearest page boundary.
     enum class Loc {
         // Refers to the inherited linked list, this includes any allocated any
         // reserved bytes, from base() to capacity_.
         //
         // This is used when freezing a LifoAlloc, such as moving a LifoAlloc to
         // another thread.
-        Header    = 0,
+        Header = 0,
         // Refers to the set of allocated and reserved bytes, from
         // PageRoundup(begin()), to capacity_.
         //
         // This is used when a BumpChunk is moved to the list of unused chunks,
         // as we want the header to remain mutable.
         Allocated = 1,
         // Refers to the set of reserved bytes, from PageRoundup(end()) to
         // capacity_.
         //
         // This is used when a BumpChunk is no longer used for allocation, while
         // containing live data. This should catch out-of-bound accesses within
         // the LifoAlloc content.
-        Reserved  = 2,
+        Reserved = 2,
         // Refers to the end of the BumpChunk.
         //
         // This is used when a BumpChunk is used for doing allocation, as
         // re-protecting at each setBump would be too costly.
         End = 3
     };
 #ifdef LIFO_CHUNK_PROTECT
     void setRWUntil(Loc loc) const;
@@ -496,26 +481,43 @@ class BumpChunk : public SingleLinkedLis
     void removeMProtectHandler() const;
 #else
     void setRWUntil(Loc loc) const {}
     void addMProtectHandler() const {}
     void removeMProtectHandler() const {}
 #endif
 };
 
+// Space reserved for the BumpChunk internal data, and the alignment of the
+// first allocation content. This can be used to ensure there is enough space
+// for the next allocation (see LifoAlloc::newChunkWithCapacity).
+static constexpr size_t BumpChunkReservedSpace = AlignBytes(sizeof(BumpChunk), LIFO_ALLOC_ALIGN);
+
+inline const uint8_t*
+BumpChunk::begin() const
+{
+    return base() + BumpChunkReservedSpace;
+}
+
+inline uint8_t*
+BumpChunk::begin()
+{
+    return base() + BumpChunkReservedSpace;
+}
+
 } // namespace detail
 
 // LIFO bump allocator: used for phase-oriented and fast LIFO allocations.
 //
 // Note: We leave BumpChunks latent in the set of unused chunks after they've
 // been released to avoid thrashing before a GC.
 class LifoAlloc
 {
     using Loc = detail::BumpChunk::Loc;
-    using BumpChunk = js::UniquePtr<detail::BumpChunk>;
+    using UniqueBumpChunk = js::UniquePtr<detail::BumpChunk>;
     using BumpChunkList = detail::SingleLinkedList<detail::BumpChunk>;
 
     // List of chunks containing allocated data. In the common case, the last
     // chunk of this list is always used to perform the allocations. When the
     // allocation cannot be performed, we move a Chunk from the unused set to
     // the list of used chunks.
     BumpChunkList chunks_;
 
@@ -532,36 +534,23 @@ class LifoAlloc
 #ifdef LIFO_CHUNK_PROTECT
     const bool  protect_;
 #endif
 
     void operator=(const LifoAlloc&) = delete;
     LifoAlloc(const LifoAlloc&) = delete;
 
     // Return a BumpChunk that can perform an allocation of at least size |n|.
-    BumpChunk newChunkWithCapacity(size_t n);
+    UniqueBumpChunk newChunkWithCapacity(size_t n);
 
     // Reuse or allocate a BumpChunk that can perform an allocation of at least
     // size |n|, if successful it is placed at the end the list of |chunks_|.
     MOZ_MUST_USE bool getOrCreateChunk(size_t n);
 
-    void reset(size_t defaultChunkSize) {
-        MOZ_ASSERT(mozilla::RoundUpPow2(defaultChunkSize) == defaultChunkSize);
-        while (!chunks_.empty()) {
-            chunks_.begin()->setRWUntil(Loc::End);
-            chunks_.popFirst();
-        }
-        while (!unused_.empty()) {
-            unused_.begin()->setRWUntil(Loc::End);
-            unused_.popFirst();
-        }
-        defaultChunkSize_ = defaultChunkSize;
-        markCount = 0;
-        curSize_ = 0;
-    }
+    void reset(size_t defaultChunkSize);
 
     // Append unused chunks to the end of this LifoAlloc.
     void appendUnused(BumpChunkList&& otherUnused) {
 #ifdef DEBUG
         for (detail::BumpChunk& bc: otherUnused)
             MOZ_ASSERT(bc.empty());
 #endif
         unused_.appendAll(std::move(otherUnused));
@@ -700,17 +689,17 @@ class LifoAlloc
         }
 
         for (detail::BumpChunk& bc : unused_) {
             total += bc.unused();
             if (total >= n)
                 return true;
         }
 
-        BumpChunk newChunk = newChunkWithCapacity(n);
+        UniqueBumpChunk newChunk = newChunkWithCapacity(n);
         if (!newChunk)
             return false;
         size_t size = newChunk->computedSizeOfIncludingThis();
         newChunk->setRWUntil(Loc::Allocated);
         unused_.pushFront(std::move(newChunk));
         incrementCurSize(size);
         return true;
     }
--- a/js/src/jsutil.h
+++ b/js/src/jsutil.h
@@ -148,28 +148,27 @@ Min(T t1, T t2)
 template <class T>
 static inline T
 Max(T t1, T t2)
 {
     return t1 > t2 ? t1 : t2;
 }
 
 template <typename T, typename U>
-static inline U
+static constexpr U
 ComputeByteAlignment(T bytes, U alignment)
 {
     static_assert(mozilla::IsUnsigned<U>::value,
                   "alignment amount must be unsigned");
 
-    MOZ_ASSERT(mozilla::IsPowerOfTwo(alignment));
     return (alignment - (bytes % alignment)) % alignment;
 }
 
 template <typename T, typename U>
-static inline T
+static constexpr T
 AlignBytes(T bytes, U alignment)
 {
     static_assert(mozilla::IsUnsigned<U>::value,
                   "alignment amount must be unsigned");
 
     return bytes + ComputeByteAlignment(bytes, alignment);
 }