From 593cb94d1ce6f68a5b616cbfe3f4b69dc70832c3 Mon Sep 17 00:00:00 2001 From: Herb Derby Date: Thu, 19 Jan 2017 14:28:49 -0500 Subject: Fix reset and deleting behavior. * Reset the Arena state. * Call all the dtors before deleting the blocks. Change-Id: I6d90463966ac7bf9f0a4fda229f67d508c86bebb Reviewed-on: https://skia-review.googlesource.com/7308 Reviewed-by: Herb Derby Commit-Queue: Herb Derby --- src/core/SkArenaAlloc.cpp | 104 ++++++++++++++++++++++++++-------------------- src/core/SkArenaAlloc.h | 86 +++++++++++++------------------------- 2 files changed, 89 insertions(+), 101 deletions(-) (limited to 'src') diff --git a/src/core/SkArenaAlloc.cpp b/src/core/SkArenaAlloc.cpp index d6c249b573..1d359d182c 100644 --- a/src/core/SkArenaAlloc.cpp +++ b/src/core/SkArenaAlloc.cpp @@ -6,46 +6,71 @@ */ #include +#include #include "SkArenaAlloc.h" -struct Skipper { - char* operator()(char* objEnd, ptrdiff_t size) { return objEnd + size; } -}; +static char* end_chain(char*) { return nullptr; } + +char* SkArenaAlloc::SkipPod(char* footerEnd) { + char* objEnd = footerEnd - (sizeof(Footer) + sizeof(int32_t)); + int32_t skip; + memmove(&skip, objEnd, sizeof(int32_t)); + return objEnd - skip; +} + +void SkArenaAlloc::RunDtorsOnBlock(char* footerEnd) { + while (footerEnd != nullptr) { + Footer footer; + memcpy(&footer, footerEnd - sizeof(Footer), sizeof(Footer)); + + FooterAction* action = (FooterAction*)((char*)end_chain + (footer >> 5)); + ptrdiff_t padding = footer & 31; -struct NextBlock { - char* operator()(char* objEnd, ptrdiff_t size) { delete [] objEnd; return objEnd + size; } + footerEnd = action(footerEnd) - padding; + } +} + +char* SkArenaAlloc::NextBlock(char* footerEnd) { + char* objEnd = footerEnd - (sizeof(Footer) + sizeof(char*)); + char* next; + memmove(&next, objEnd, sizeof(char*)); + RunDtorsOnBlock(next); + delete [] objEnd; + return nullptr; +} + +struct Skipper { + char* operator()(char* objEnd, uint32_t size) { return objEnd - size; } }; SkArenaAlloc::SkArenaAlloc(char* block, size_t size, size_t extraSize) - : fDtorCursor{block} - , fCursor {block} - , fEnd {block + size} - , fExtraSize {extraSize} + : fDtorCursor {block} + , fCursor {block} + , fEnd {block + size} + , fFirstBlock {block} + , fFirstSize {size} + , fExtraSize {extraSize} { if (size < sizeof(Footer)) { fEnd = fCursor = fDtorCursor = nullptr; } if (fCursor != nullptr) { - this->installFooter(EndChain, 0); + this->installFooter(end_chain, 0); } } SkArenaAlloc::~SkArenaAlloc() { - this->reset(); + RunDtorsOnBlock(fDtorCursor); } void SkArenaAlloc::reset() { - Footer f; - memmove(&f, fDtorCursor - sizeof(Footer), sizeof(Footer)); - char* releaser = fDtorCursor; - while (releaser != nullptr) { - releaser = this->callFooterAction(releaser); - } + this->~SkArenaAlloc(); + new (this) SkArenaAlloc{fFirstBlock, fFirstSize, fExtraSize}; } void SkArenaAlloc::installFooter(FooterAction* releaser, ptrdiff_t padding) { - ptrdiff_t releaserDiff = (char *)releaser - (char *)EndChain; + ptrdiff_t releaserDiff = (char *)releaser - (char *)end_chain; ptrdiff_t footerData = SkLeftShift((int64_t)releaserDiff, 5) | padding; if (padding >= 32 || !SkTFitsIn(footerData)) { // Footer data will not fit. @@ -54,12 +79,22 @@ void SkArenaAlloc::installFooter(FooterAction* releaser, ptrdiff_t padding) { Footer footer = (Footer)(footerData); memmove(fCursor, &footer, sizeof(Footer)); - Footer check; - memmove(&check, fCursor, sizeof(Footer)); fCursor += sizeof(Footer); fDtorCursor = fCursor; } +void SkArenaAlloc::installPtrFooter(FooterAction* action, char* ptr, ptrdiff_t padding) { + memmove(fCursor, &ptr, sizeof(char*)); + fCursor += sizeof(char*); + this->installFooter(action, padding); +} + +void SkArenaAlloc::installUint32Footer(FooterAction* action, uint32_t value, ptrdiff_t padding) { + memmove(fCursor, &value, sizeof(uint32_t)); + fCursor += sizeof(uint32_t); + this->installFooter(action, padding); +} + void SkArenaAlloc::ensureSpace(size_t size, size_t alignment) { constexpr size_t headerSize = sizeof(Footer) + sizeof(ptrdiff_t); // The chrome c++ library we use does not define std::max_align_t. @@ -76,7 +111,7 @@ void SkArenaAlloc::ensureSpace(size_t size, size_t alignment) { // Round up to a nice size. If > 32K align to 4K boundary else up to max_align_t. The > 32K // heuristic is from the JEMalloc behavior. { - size_t mask = allocationSize > (1 << 15) ? (1 << 12) - 1 : 32 - 1; + size_t mask = allocationSize > (1 << 15) ? (1 << 12) - 1 : 16 - 1; allocationSize = (allocationSize + mask) & ~mask; } @@ -86,7 +121,7 @@ void SkArenaAlloc::ensureSpace(size_t size, size_t alignment) { fCursor = newBlock; fDtorCursor = newBlock; fEnd = fCursor + allocationSize; - this->installIntFooter(previousDtor - fCursor, 0); + this->installPtrFooter(NextBlock, previousDtor, 0); } char* SkArenaAlloc::allocObject(size_t size, size_t alignment) { @@ -99,19 +134,14 @@ char* SkArenaAlloc::allocObject(size_t size, size_t alignment) { return objStart; } -// * sizeAndFooter - the memory for the footer in addition to the size for the object. -// * alignment - alignment needed by the object. char* SkArenaAlloc::allocObjectWithFooter(size_t sizeIncludingFooter, size_t alignment) { size_t mask = alignment - 1; - restart: +restart: size_t skipOverhead = 0; bool needsSkipFooter = fCursor != fDtorCursor; if (needsSkipFooter) { - size_t skipSize = SkTFitsIn(fDtorCursor - fCursor) - ? sizeof(int32_t) - : sizeof(ptrdiff_t); - skipOverhead = sizeof(Footer) + skipSize; + skipOverhead = sizeof(Footer) + sizeof(uint32_t); } char* objStart = (char*)((uintptr_t)(fCursor + skipOverhead + mask) & ~mask); size_t totalSize = sizeIncludingFooter + skipOverhead; @@ -126,23 +156,9 @@ char* SkArenaAlloc::allocObjectWithFooter(size_t sizeIncludingFooter, size_t ali // Install a skip footer if needed, thus terminating a run of POD data. The calling code is // responsible for installing the footer after the object. if (needsSkipFooter) { - this->installIntFooter(fDtorCursor - fCursor, 0); + this->installUint32Footer(SkipPod, SkTo(fCursor - fDtorCursor), 0); } return objStart; } -char* SkArenaAlloc::callFooterAction(char* end) { - Footer footer; - memcpy(&footer, end - sizeof(Footer), sizeof(Footer)); - - FooterAction* releaser = (FooterAction*)((char*)EndChain + (footer >> 5)); - ptrdiff_t padding = footer & 31; - - char* r = releaser(end) - padding; - - return r; -} - -char* SkArenaAlloc::EndChain(char*) { return nullptr; } - diff --git a/src/core/SkArenaAlloc.h b/src/core/SkArenaAlloc.h index 8152c94cbd..7deac72bd8 100644 --- a/src/core/SkArenaAlloc.h +++ b/src/core/SkArenaAlloc.h @@ -56,7 +56,7 @@ public: SkArenaAlloc(char* block, size_t size, size_t extraSize = 0); template - SkArenaAlloc(char (&block)[kSize], size_t extraSize = 0) + SkArenaAlloc(char (&block)[kSize], size_t extraSize = kSize) : SkArenaAlloc(block, kSize, extraSize) {} @@ -116,38 +116,13 @@ private: using Footer = int32_t; using FooterAction = char* (char*); - void installFooter(FooterAction* releaser, ptrdiff_t padding); + static char* SkipPod(char* footerEnd); + static void RunDtorsOnBlock(char* footerEnd); + static char* NextBlock(char* footerEnd); - // N.B. Action is different than FooterAction. FooterAction expects the end of the Footer, - // and returns the start of the object. An Action expects the end of the *Object* and returns - // the start of the object. - template - void installIntFooter(ptrdiff_t size, ptrdiff_t padding) { - if (SkTFitsIn(size)) { - int32_t smallSize = static_cast(size); - memmove(fCursor, &smallSize, sizeof(int32_t)); - fCursor += sizeof(int32_t); - this->installFooter( - [](char* footerEnd) { - char* objEnd = footerEnd - (sizeof(Footer) + sizeof(int32_t)); - int32_t data; - memmove(&data, objEnd, sizeof(int32_t)); - return Action()(objEnd, data); - }, - padding); - } else { - memmove(fCursor, &size, sizeof(ptrdiff_t)); - fCursor += sizeof(ptrdiff_t); - this->installFooter( - [](char* footerEnd) { - char* objEnd = footerEnd - (sizeof(Footer) + sizeof(ptrdiff_t)); - ptrdiff_t data; - memmove(&data, objEnd, sizeof(ptrdiff_t)); - return Action()(objEnd, data); - }, - padding); - } - } + void installFooter(FooterAction* releaser, ptrdiff_t padding); + void installUint32Footer(FooterAction* action, uint32_t value, ptrdiff_t padding); + void installPtrFooter(FooterAction* action, char* ptr, ptrdiff_t padding); void ensureSpace(size_t size, size_t alignment); @@ -157,48 +132,45 @@ private: template char* commonArrayAlloc(size_t count) { + SkASSERT(SkTFitsIn(count)); char* objStart; size_t arraySize = count * sizeof(T); - SkASSERT(arraySize > 0); - if (skstd::is_trivially_destructible::value) { objStart = this->allocObject(arraySize, alignof(T)); fCursor = objStart + arraySize; } else { - size_t countSize = SkTFitsIn(count) ? sizeof(int32_t) : sizeof(ptrdiff_t); - size_t totalSize = arraySize + sizeof(Footer) + countSize; + size_t totalSize = arraySize + sizeof(Footer) + sizeof(uint32_t); objStart = this->allocObjectWithFooter(totalSize, alignof(T)); size_t padding = objStart - fCursor; // Advance to end of array to install footer.? fCursor = objStart + arraySize; - this->installIntFooter> (count, padding); + this->installUint32Footer( + [](char* footerEnd) { + char* objEnd = footerEnd - (sizeof(Footer) + sizeof(uint32_t)); + uint32_t count; + memmove(&count, objEnd, sizeof(uint32_t)); + char* objStart = objEnd - count * sizeof(T); + T* array = (T*) objStart; + for (uint32_t i = 0; i < count; i++) { + array[i].~T(); + } + return objStart; + }, + SkTo(count), + padding); } return objStart; } - char* callFooterAction(char* end); - - static char* EndChain(char*); - - template - struct ArrayDestructor { - char* operator()(char* objEnd, ptrdiff_t count) { - char* objStart = objEnd - count * sizeof(T); - T* array = (T*) objStart; - for (int i = 0; i < count; i++) { - array[i].~T(); - } - return objStart; - } - }; - - char* fDtorCursor; - char* fCursor; - char* fEnd; - size_t fExtraSize; + char* fDtorCursor; + char* fCursor; + char* fEnd; + char* const fFirstBlock; + const size_t fFirstSize; + const size_t fExtraSize; }; #endif//SkFixedAlloc_DEFINED -- cgit v1.2.3