Keep a scratch block around for reuse in GrBlockAllocator

This can optimize cases in a allocate-release loop that moves back
and forth across the end of one block and the start of another. Before
this would malloc a new block and then delete it.

It also enables reserve() in higher-level data collections w/o blocking
bytes left in the current tail block.

Change-Id: Ide16e9038384fcb188164fc9620a8295f6880b9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303268
Reviewed-by: Brian Salomon <[email protected]>
Reviewed-by: Robert Phillips <[email protected]>
Commit-Queue: Michael Ludwig <[email protected]>

Author: lhkbob

src/gpu/GrBlockAllocator.cpp

@@ -46,7 +46,7 @@ GrBlockAllocator::Block::~Block() {
 
 size_t GrBlockAllocator::totalSize() const {
     // Use size_t since the sum across all blocks could exceed 'int', even though each block won't
-    size_t size = offsetof(GrBlockAllocator, fHead);
+    size_t size = offsetof(GrBlockAllocator, fHead) + this->scratchBlockSize();
     for (const Block* b : this->blocks()) {
         size += b->fSize;
     }
@@ -55,7 +55,10 @@ size_t GrBlockAllocator::totalSize() const {
 }
 
 size_t GrBlockAllocator::totalUsableSpace() const {
-    size_t size = 0;
+    size_t size = this->scratchBlockSize();
+    if (size > 0) {
+        size -= kDataStart; // scratchBlockSize reports total block size, not usable size
+    }
     for (const Block* b : this->blocks()) {
         size += (b->fSize - kDataStart);
     }
@@ -87,9 +90,14 @@ GrBlockAllocator::Block* GrBlockAllocator::findOwningBlock(const void* p) {
 }
 
 void GrBlockAllocator::releaseBlock(Block* block) {
-    if (block->fPrev) {
-        // Unlink block from the double-linked list of blocks
-        SkASSERT(block != &fHead);
+     if (block == &fHead) {
+        // Reset the cursor of the head block so that it can be reused if it becomes the new tail
+        block->fCursor = kDataStart;
+        block->fMetadata = 0;
+        // Unlike in reset(), we don't set the head's next block to null because there are
+        // potentially heap-allocated blocks that are still connected to it.
+    } else {
+        SkASSERT(block->fPrev);
         block->fPrev->fNext = block->fNext;
         if (block->fNext) {
             SkASSERT(fTail != block);
@@ -99,14 +107,17 @@ void GrBlockAllocator::releaseBlock(Block* block) {
             fTail = block->fPrev;
         }
 
-        delete block;
-    } else {
-        // Reset the cursor of the head block so that it can be reused
-        SkASSERT(block == &fHead);
-        block->fCursor = kDataStart;
-        block->fMetadata = 0;
-        // Unlike in reset(), we don't set the head's next block to null because there are
-        // potentially heap-allocated blocks that are still connected to it.
+        // The released block becomes the new scratch block (if it's bigger), or delete it
+        if (this->scratchBlockSize() < block->fSize) {
+            SkASSERT(block != fHead.fPrev); // sanity check, shouldn't already be the scratch block
+            if (fHead.fPrev) {
+                delete fHead.fPrev;
+            }
+            block->markAsScratch();
+            fHead.fPrev = block;
+        } else {
+            delete block;
+        }
     }
 
     // Decrement growth policy (opposite of addBlock()'s increment operations)
@@ -139,62 +150,89 @@ void GrBlockAllocator::reset() {
             b->fNext = nullptr;
             b->fCursor = kDataStart;
             b->fMetadata = 0;
-
-            // For reset(), but NOT releaseBlock(), the head allocatorMetadata resets too
+            // For reset(), but NOT releaseBlock(), the head allocatorMetadata and scratch block
+            // are reset/destroyed.
             b->fAllocatorMetadata = 0;
+            this->resetScratchSpace();
         } else {
             delete b;
         }
     }
-    SkASSERT(fTail == &fHead && fHead.fNext == nullptr &&
+    SkASSERT(fTail == &fHead && fHead.fNext == nullptr && fHead.fPrev == nullptr &&
              fHead.metadata() == 0 && fHead.fCursor == kDataStart);
 
     GrowthPolicy gp = static_cast<GrowthPolicy>(fGrowthPolicy);
     fN0 = (gp == GrowthPolicy::kLinear || gp == GrowthPolicy::kExponential) ? 1 : 0;
     fN1 = 1;
 }
 
+void GrBlockAllocator::resetScratchSpace() {
+    if (fHead.fPrev) {
+        delete fHead.fPrev;
+        fHead.fPrev = nullptr;
+    }
+}
+
 void GrBlockAllocator::addBlock(int minimumSize, int maxSize) {
     SkASSERT(minimumSize > (int) sizeof(Block) && minimumSize <= maxSize);
 
     // Max positive value for uint:23 storage (decltype(fN0) picks up uint64_t, not uint:23).
     static constexpr int kMaxN = (1 << 23) - 1;
     static_assert(2 * kMaxN <= std::numeric_limits<int32_t>::max()); // Growth policy won't overflow
 
-    // Calculate the 'next' size per growth policy sequence
-    GrowthPolicy gp = static_cast<GrowthPolicy>(fGrowthPolicy);
-    int nextN1 = fN0 + fN1;
-    int nextN0;
-    if (gp == GrowthPolicy::kFixed || gp == GrowthPolicy::kLinear) {
-        nextN0 = fN0;
-    } else if (gp == GrowthPolicy::kFibonacci) {
-        nextN0 = fN1;
-    } else {
-        SkASSERT(gp == GrowthPolicy::kExponential);
-        nextN0 = nextN1;
-    }
-    fN0 = std::min(kMaxN, nextN0);
-    fN1 = std::min(kMaxN, nextN1);
+    auto alignAllocSize = [](int size) {
+        // Round to a nice boundary since the block isn't maxing out:
+        //   if allocSize > 32K, aligns on 4K boundary otherwise aligns on max_align_t, to play
+        //   nicely with jeMalloc (from SkArenaAlloc).
+        int mask = size > (1 << 15) ? ((1 << 12) - 1) : (kAddressAlign - 1);
+        return (size + mask) & ~mask;
+    };
 
-    // However, must guard against overflow here, since all the size-based asserts prevented
-    // alignment/addition overflows, while multiplication requires 2x bits instead of x+1.
-    int sizeIncrement = fBlockIncrement * kAddressAlign;
     int allocSize;
-    if (maxSize / sizeIncrement < nextN1) {
-        // The growth policy would overflow, so use the max. We've already confirmed that maxSize
-        // will be sufficient for the requested minimumSize
-        allocSize = maxSize;
+    void* mem = nullptr;
+    if (this->scratchBlockSize() >= minimumSize) {
+        // Activate the scratch block instead of making a new block
+        SkASSERT(fHead.fPrev->isScratch());
+        allocSize = fHead.fPrev->fSize;
+        mem = fHead.fPrev;
+        fHead.fPrev = nullptr;
+    } else if (minimumSize < maxSize) {
+        // Calculate the 'next' size per growth policy sequence
+        GrowthPolicy gp = static_cast<GrowthPolicy>(fGrowthPolicy);
+        int nextN1 = fN0 + fN1;
+        int nextN0;
+        if (gp == GrowthPolicy::kFixed || gp == GrowthPolicy::kLinear) {
+            nextN0 = fN0;
+        } else if (gp == GrowthPolicy::kFibonacci) {
+            nextN0 = fN1;
+        } else {
+            SkASSERT(gp == GrowthPolicy::kExponential);
+            nextN0 = nextN1;
+        }
+        fN0 = std::min(kMaxN, nextN0);
+        fN1 = std::min(kMaxN, nextN1);
+
+        // However, must guard against overflow here, since all the size-based asserts prevented
+        // alignment/addition overflows, while multiplication requires 2x bits instead of x+1.
+        int sizeIncrement = fBlockIncrement * kAddressAlign;
+        if (maxSize / sizeIncrement < nextN1) {
+            // The growth policy would overflow, so use the max. We've already confirmed that
+            // maxSize will be sufficient for the requested minimumSize
+            allocSize = maxSize;
+        } else {
+            allocSize = std::min(alignAllocSize(std::max(minimumSize, sizeIncrement * nextN1)),
+                                 maxSize);
+        }
     } else {
-        allocSize = std::max(minimumSize, sizeIncrement * nextN1);
-        // Then round to a nice boundary since the block isn't maxing out:
-        //   if allocSize > 32K, aligns on 4K boundary otherwise aligns on max_align_t, to play
-        //   nicely with jeMalloc (from SkArenaAlloc).
-        int mask = allocSize > (1 << 15) ? ((1 << 12) - 1) : (kAddressAlign - 1);
-        allocSize = std::min((allocSize + mask) & ~mask, maxSize);
+        SkASSERT(minimumSize == maxSize);
+        // Still align on a nice boundary, no max clamping since that would just undo the alignment
+        allocSize = alignAllocSize(minimumSize);
     }
 
     // Create new block and append to the linked list of blocks in this allocator
-    void* mem = operator new(allocSize);
+    if (!mem) {
+        mem = operator new(allocSize);
+    }
     fTail->fNext = new (mem) Block(fTail, allocSize);
     fTail = fTail->fNext;
 }
@@ -207,7 +245,13 @@ void GrBlockAllocator::validate() const {
         blocks.push_back(block);
 
         SkASSERT(kAssignedMarker == block->fSentinel);
-        SkASSERT(prev == block->fPrev);
+        if (block == &fHead) {
+            // The head blocks' fPrev may be non-null if it holds a scratch block, but that's not
+            // considered part of the linked list
+            SkASSERT(!prev && (!fHead.fPrev || fHead.fPrev->isScratch()));
+        } else {
+            SkASSERT(prev == block->fPrev);
+        }
         if (prev) {
             SkASSERT(prev->fNext == block);
         }

src/gpu/GrBlockAllocator.h

@@ -132,6 +132,9 @@ class GrBlockAllocator final : SkNoncopyable {
         template <size_t Align, size_t Padding>
         int alignedOffset(int offset) const;
 
+        bool isScratch() const { return fCursor < 0; }
+        void markAsScratch() { fCursor = -1; }
+
         SkDEBUGCODE(int fSentinel;) // known value to check for bad back pointers to blocks
 
         Block*          fNext;      // doubly-linked list of blocks
@@ -259,6 +262,33 @@ class GrBlockAllocator final : SkNoncopyable {
     template <size_t Align, size_t Padding = 0>
     ByteRange allocate(size_t size);
 
+    enum ReserveFlags : unsigned {
+        // If provided to reserve(), the input 'size' will be rounded up to the next size determined
+        // by the growth policy of the GrBlockAllocator. If not, 'size' will be aligned to max_align
+        kIgnoreGrowthPolicy_Flag  = 0b01,
+        // If provided to reserve(), the number of available bytes of the current block  will not
+        // be used to satisfy the reservation (assuming the contiguous range was long enough to
+        // begin with).
+        kIgnoreExistingBytes_Flag = 0b10,
+
+        kNo_ReserveFlags          = 0b00
+    };
+
+    /**
+     * Ensure the block allocator has 'size' contiguous available bytes. After calling this
+     * function, currentBlock()->avail<Align, Padding>() may still report less than 'size' if the
+     * reserved space was added as a scratch block. This is done so that anything remaining in
+     * the current block can still be used if a smaller-than-size allocation is requested. If 'size'
+     * is requested by a subsequent allocation, the scratch block will automatically be activated
+     * and the request will not itself trigger any malloc.
+     *
+     * The optional 'flags' controls how the input size is allocated; by default it will attempt
+     * to use available contiguous bytes in the current block and will respect the growth policy
+     * of the allocator.
+     */
+    template <size_t Align = 1, size_t Padding = 0>
+    void reserve(size_t size, ReserveFlags flags = kNo_ReserveFlags);
+
     /**
      * Return a pointer to the start of the current block. This will never be null.
      */
@@ -305,6 +335,10 @@ class GrBlockAllocator final : SkNoncopyable {
      *
      * If 'block' represents the inline-allocated head block, its cursor and metadata are instead
      * reset to their defaults.
+     *
+     * If the block is not the head block, it may be kept as a scratch block to be reused for
+     * subsequent allocation requests, instead of making an entirely new block. A scratch block is
+     * not visible when iterating over blocks but is reported in the total size of the allocator.
      */
     void releaseBlock(Block* block);
 
@@ -314,6 +348,11 @@ class GrBlockAllocator final : SkNoncopyable {
      */
     void reset();
 
+    /**
+     * Remove any reserved scratch space, either from calling reserve() or releaseBlock().
+     */
+    void resetScratchSpace();
+
     template <bool Forward, bool Const> class BlockIter;
 
     /**
@@ -338,6 +377,10 @@ class GrBlockAllocator final : SkNoncopyable {
     void validate() const;
 #endif
 
+#if GR_TEST_UTILS
+    int testingOnly_scratchBlockSize() const { return this->scratchBlockSize(); }
+#endif
+
 private:
     static constexpr int kDataStart = sizeof(Block);
     #ifdef SK_FORCE_8_BYTE_ALIGNMENT
@@ -369,6 +412,8 @@ class GrBlockAllocator final : SkNoncopyable {
     // that will preserve the static guarantees GrBlockAllocator makes.
     void addBlock(int minSize, int maxSize);
 
+    int scratchBlockSize() const { return fHead.fPrev ? fHead.fPrev->fSize : 0; }
+
     Block* fTail; // All non-head blocks are heap allocated; tail will never be null.
 
     // All remaining state is packed into 64 bits to keep GrBlockAllocator at 16 bytes + head block
@@ -390,6 +435,9 @@ class GrBlockAllocator final : SkNoncopyable {
 
     // Inline head block, must be at the end so that it can utilize any additional reserved space
     // from the initial allocation.
+    // The head block's prev pointer may be non-null, which signifies a scratch block that may be
+    // reused instead of allocating an entirely new block (this helps when allocate+release calls
+    // bounce back and forth across the capacity of a block).
     alignas(kAddressAlign) Block fHead;
 
     static_assert(kGrowthPolicyCount <= 4);
@@ -435,6 +483,8 @@ class GrSBlockAllocator : SkNoncopyable {
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 // Template and inline implementations
 
+GR_MAKE_BITFIELD_OPS(GrBlockAllocator::ReserveFlags)
+
 template<size_t Align, size_t Padding>
 constexpr size_t GrBlockAllocator::BlockOverhead() {
     static_assert(GrAlignTo(kDataStart + Padding, Align) >= sizeof(Block));
@@ -457,6 +507,29 @@ constexpr size_t GrBlockAllocator::MaxBlockSize() {
     return BlockOverhead<Align, Padding>() + kMaxAllocationSize;
 }
 
+template<size_t Align, size_t Padding>
+void GrBlockAllocator::reserve(size_t size, ReserveFlags flags) {
+    if (size > kMaxAllocationSize) {
+        SK_ABORT("Allocation too large (%zu bytes requested)", size);
+    }
+    int iSize = (int) size;
+    if ((flags & kIgnoreExistingBytes_Flag) ||
+        this->currentBlock()->avail<Align, Padding>() < iSize) {
+
+        int blockSize = BlockOverhead<Align, Padding>() + iSize;
+        int maxSize = (flags & kIgnoreGrowthPolicy_Flag) ? blockSize
+                                                         : MaxBlockSize<Align, Padding>();
+        SkASSERT((size_t) maxSize <= (MaxBlockSize<Align, Padding>()));
+
+        SkDEBUGCODE(auto oldTail = fTail;)
+        this->addBlock(blockSize, maxSize);
+        SkASSERT(fTail != oldTail);
+        // Releasing the just added block will move it into scratch space, allowing the original
+        // tail's bytes to be used first before the scratch block is activated.
+        this->releaseBlock(fTail);
+    }
+}
+
 template <size_t Align, size_t Padding>
 GrBlockAllocator::ByteRange GrBlockAllocator::allocate(size_t size) {
     // Amount of extra space for a new block to make sure the allocation can succeed.
@@ -599,6 +672,12 @@ class GrBlockAllocator::BlockIter {
         void advance(BlockT* block) {
             fBlock = block;
             fNext = block ? (Forward ? block->fNext : block->fPrev) : nullptr;
+            if (!Forward && fNext && fNext->isScratch()) {
+                // For reverse-iteration only, we need to stop at the head, not the scratch block
+                // possibly stashed in head->prev.
+                fNext = nullptr;
+            }
+            SkASSERT(!fNext || !fNext->isScratch());
         }
 
         BlockT* fBlock;

src/gpu/GrMemoryPool.h

@@ -67,7 +67,8 @@ class GrMemoryPool {
     bool isEmpty() const {
         // If size is the same as preallocSize, there aren't any heap blocks, so currentBlock()
         // is the inline head block.
-        return 0 == this->size() && 0 == fAllocator.currentBlock()->metadata();
+        return fAllocator.currentBlock() == fAllocator.headBlock() &&
+               fAllocator.currentBlock()->metadata() == 0;
     }
 
     /**