Revert "Fix reset and deleting behavior."

This reverts commit 412a86d. Reason for revert: <INSERT REASONING HERE> Original change's description: > Fix reset and deleting behavior. > > * Reset the Arena state. > * Call all the dtors before deleting the blocks. > > [email protected] > > Change-Id: Iac320fec16e572cc9a6184c1f580089ab720f036 > Reviewed-on: https://skia-review.googlesource.com/7221 > Reviewed-by: Herb Derby <[email protected]> > Commit-Queue: Herb Derby <[email protected]> > [email protected] NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I4f4d34e0190a60d418f11326a9a9688d7487b8d8 Reviewed-on: https://skia-review.googlesource.com/7261 Reviewed-by: Herb Derby <[email protected]> Commit-Queue: Herb Derby <[email protected]>
jasonborn · Jan 19, 2017 · 1517224 · 1517224
1 parent 412a86d
commit 1517224
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 85 deletions.
diff --git 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 @@ char* SkArenaAlloc::allocObjectWithFooter(size_t sizeIncludingFooter, size_t ali
     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
@@ -56,7  56,7 @@ class SkArenaAlloc {
     SkArenaAlloc(char* block, size_t size, size_t extraSize = 0);
 
     template <size_t kSize>
-    SkArenaAlloc(char (&block)[kSize], size_t extraSize = kSize)
     SkArenaAlloc(char (&block)[kSize], size_t extraSize = 0)
         : SkArenaAlloc(block, kSize, extraSize)
     {}
 
@@ -116,8  116,6 @@ class SkArenaAlloc {
     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 @@ class SkArenaAlloc {
         return objStart;
     }
 
-    static char* CallFooterAction(char* end);
-
-    static void ResetBlock(char* footerEnd);
     char* callFooterAction(char* end);
 
     static char* EndChain(char*);
 
@@ -199,12  195,10 @@ class SkArenaAlloc {
         }
     };
 
-    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
@@ -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<int>(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<char>(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<uint64_t>(sizeof(Node) / sizeof(Node*));
-            current = new (temp)Node(current);
-        }
-        start.start = current;
-    }
-
-    REPORTER_ASSERT(r, created == 128);
-    REPORTER_ASSERT(r, destroyed == 128);
 }