From 1517224346e9e3e017401ec3f92bf30d98e5e995 Mon Sep 17 00:00:00 2001 From: Herb Derby Date: Thu, 19 Jan 2017 03:32:23 +0000 Subject: [PATCH] Revert "Fix reset and deleting behavior." This reverts commit 412a86d014783be99a7a9a0fae407791b95806e8. Reason for revert: Original change's description: > Fix reset and deleting behavior. > > * Reset the Arena state. > * Call all the dtors before deleting the blocks. > > TBR=mtklein@google.com > > Change-Id: Iac320fec16e572cc9a6184c1f580089ab720f036 > Reviewed-on: https://skia-review.googlesource.com/7221 > Reviewed-by: Herb Derby > Commit-Queue: Herb Derby > TBR=herb@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I4f4d34e0190a60d418f11326a9a9688d7487b8d8 Reviewed-on: https://skia-review.googlesource.com/7261 Reviewed-by: Herb Derby Commit-Queue: Herb Derby --- src/core/SkArenaAlloc.cpp | 46 ++++++++++++++++--------------------- src/core/SkArenaAlloc.h | 18 +++++---------- tests/ArenaAllocTest.cpp | 48 +-------------------------------------- 3 files changed, 27 insertions(+), 85 deletions(-) diff --git a/src/core/SkArenaAlloc.cpp b/src/core/SkArenaAlloc.cpp index 03ab9fad4f..d6c249b573 100644 --- a/src/core/SkArenaAlloc.cpp +++ b/src/core/SkArenaAlloc.cpp @@ -12,21 +12,15 @@ struct Skipper { char* operator()(char* objEnd, ptrdiff_t size) { return objEnd + size; } }; -struct SkArenaAlloc::NextBlock { - char* operator()(char* objEnd, ptrdiff_t size) { - ResetBlock(objEnd + size); - delete [] objEnd; - return nullptr; - } +struct NextBlock { + char* operator()(char* objEnd, ptrdiff_t size) { delete [] objEnd; return objEnd + size; } }; SkArenaAlloc::SkArenaAlloc(char* block, size_t size, size_t extraSize) - : fDtorCursor {block} - , fCursor {block} - , fEnd {block + size} - , fFirstBlock {block} - , fFirstSize {size} - , fExtraSize {extraSize} + : fDtorCursor{block} + , fCursor {block} + , fEnd {block + size} + , fExtraSize {extraSize} { if (size < sizeof(Footer)) { fEnd = fCursor = fDtorCursor = nullptr; @@ -38,12 +32,16 @@ SkArenaAlloc::SkArenaAlloc(char* block, size_t size, size_t extraSize) } SkArenaAlloc::~SkArenaAlloc() { - ResetBlock(fDtorCursor); + this->reset(); } void SkArenaAlloc::reset() { - this->~SkArenaAlloc(); - new (this) SkArenaAlloc{fFirstBlock, fFirstSize, fExtraSize}; + Footer f; + memmove(&f, fDtorCursor - sizeof(Footer), sizeof(Footer)); + char* releaser = fDtorCursor; + while (releaser != nullptr) { + releaser = this->callFooterAction(releaser); + } } void SkArenaAlloc::installFooter(FooterAction* releaser, ptrdiff_t padding) { @@ -56,6 +54,8 @@ 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; } @@ -104,7 +104,7 @@ char* SkArenaAlloc::allocObject(size_t size, size_t alignment) { 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) { @@ -132,20 +132,14 @@ restart: return objStart; } -void SkArenaAlloc::ResetBlock(char* footerEnd) { - while (footerEnd != nullptr) { - footerEnd = CallFooterAction(footerEnd); - } -} - -char* SkArenaAlloc::CallFooterAction(char* footerEnd) { +char* SkArenaAlloc::callFooterAction(char* end) { Footer footer; - memcpy(&footer, footerEnd - sizeof(Footer), sizeof(Footer)); + memcpy(&footer, end - sizeof(Footer), sizeof(Footer)); - FooterAction* action = (FooterAction*)((char*)EndChain + (footer >> 5)); + FooterAction* releaser = (FooterAction*)((char*)EndChain + (footer >> 5)); ptrdiff_t padding = footer & 31; - char* r = action(footerEnd) - padding; + char* r = releaser(end) - padding; return r; } diff --git a/src/core/SkArenaAlloc.h b/src/core/SkArenaAlloc.h index 56006c30b2..8152c94cbd 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 = kSize) + SkArenaAlloc(char (&block)[kSize], size_t extraSize = 0) : SkArenaAlloc(block, kSize, extraSize) {} @@ -116,8 +116,6 @@ private: using Footer = int32_t; using FooterAction = char* (char*); - struct NextBlock; - void installFooter(FooterAction* releaser, ptrdiff_t padding); // N.B. Action is different than FooterAction. FooterAction expects the end of the Footer, @@ -181,9 +179,7 @@ private: return objStart; } - static char* CallFooterAction(char* end); - - static void ResetBlock(char* footerEnd); + char* callFooterAction(char* end); static char* EndChain(char*); @@ -199,12 +195,10 @@ private: } }; - char* fDtorCursor; - char* fCursor; - char* fEnd; - char* const fFirstBlock; - const size_t fFirstSize; - const size_t fExtraSize; + char* fDtorCursor; + char* fCursor; + char* fEnd; + size_t fExtraSize; }; #endif//SkFixedAlloc_DEFINED diff --git a/tests/ArenaAllocTest.cpp b/tests/ArenaAllocTest.cpp index 0836b0c49f..647ea25283 100644 --- a/tests/ArenaAllocTest.cpp +++ b/tests/ArenaAllocTest.cpp @@ -26,26 +26,6 @@ namespace { uint32_t array[128]; }; - struct Node { - Node(Node* n) : next(n) { created++; } - ~Node() { - destroyed++; - if (next) { - next->~Node(); - } - } - Node *next; - }; - - struct Start { - ~Start() { - if (start) { - start->~Node(); - } - } - Node* start; - }; - } struct WithDtor { @@ -83,7 +63,7 @@ DEF_TEST(ArenaAlloc, r) { { created = 0; destroyed = 0; - char block[64]; + char block[1024]; SkArenaAlloc arena{block}; REPORTER_ASSERT(r, *arena.make(3) == 3); @@ -133,30 +113,4 @@ DEF_TEST(ArenaAlloc, r) { } REPORTER_ASSERT(r, created == 11); REPORTER_ASSERT(r, destroyed == 11); - - { - char storage[64]; - SkArenaAlloc arena{storage}; - arena.makeArrayDefault(256); - arena.reset(); - arena.reset(); - } - - { - created = 0; - destroyed = 0; - char storage[64]; - SkArenaAlloc arena{storage}; - - Start start; - Node* current = nullptr; - for (int i = 0; i < 128; i++) { - uint64_t* temp = arena.makeArrayDefault(sizeof(Node) / sizeof(Node*)); - current = new (temp)Node(current); - } - start.start = current; - } - - REPORTER_ASSERT(r, created == 128); - REPORTER_ASSERT(r, destroyed == 128); } -- 2.34.1