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 <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
diff --git a/src/core/SkArenaAlloc.cpp b/src/core/SkArenaAlloc.cpp
index d6c249b..1d359d1 100644
--- a/src/core/SkArenaAlloc.cpp
+++ b/src/core/SkArenaAlloc.cpp
@@ -6,46 +6,71 @@
*/
#include <algorithm>
+#include <cstddef>
#include "SkArenaAlloc.h"
-struct Skipper {
- char* operator()(char* objEnd, ptrdiff_t size) { return objEnd + size; }
-};
+static char* end_chain(char*) { return nullptr; }
-struct NextBlock {
- char* operator()(char* objEnd, ptrdiff_t size) { delete [] objEnd; return objEnd + size; }
+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;
+
+ 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<int32_t>(footerData)) {
// Footer data will not fit.
@@ -54,12 +79,22 @@
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 @@
// 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 @@
fCursor = newBlock;
fDtorCursor = newBlock;
fEnd = fCursor + allocationSize;
- this->installIntFooter<NextBlock>(previousDtor - fCursor, 0);
+ this->installPtrFooter(NextBlock, previousDtor, 0);
}
char* SkArenaAlloc::allocObject(size_t size, size_t alignment) {
@@ -99,19 +134,14 @@
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<int32_t>(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 @@
// 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<Skipper>(fDtorCursor - fCursor, 0);
+ this->installUint32Footer(SkipPod, SkTo<uint32_t>(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 8152c94..7deac72 100644
--- a/src/core/SkArenaAlloc.h
+++ b/src/core/SkArenaAlloc.h
@@ -56,7 +56,7 @@
SkArenaAlloc(char* block, size_t size, size_t extraSize = 0);
template <size_t kSize>
- SkArenaAlloc(char (&block)[kSize], size_t extraSize = 0)
+ SkArenaAlloc(char (&block)[kSize], size_t extraSize = kSize)
: SkArenaAlloc(block, kSize, extraSize)
{}
@@ -116,38 +116,13 @@
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<typename Action>
- void installIntFooter(ptrdiff_t size, ptrdiff_t padding) {
- if (SkTFitsIn<int32_t>(size)) {
- int32_t smallSize = static_cast<int32_t>(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 @@
template <typename T>
char* commonArrayAlloc(size_t count) {
+ SkASSERT(SkTFitsIn<uint32_t>(count));
char* objStart;
size_t arraySize = count * sizeof(T);
- SkASSERT(arraySize > 0);
-
if (skstd::is_trivially_destructible<T>::value) {
objStart = this->allocObject(arraySize, alignof(T));
fCursor = objStart + arraySize;
} else {
- size_t countSize = SkTFitsIn<int32_t>(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<ArrayDestructor<T>> (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<uint32_t>(count),
+ padding);
}
return objStart;
}
- char* callFooterAction(char* end);
-
- static char* EndChain(char*);
-
- template<typename T>
- 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
diff --git a/tests/ArenaAllocTest.cpp b/tests/ArenaAllocTest.cpp
index 647ea25..0836b0c 100644
--- a/tests/ArenaAllocTest.cpp
+++ b/tests/ArenaAllocTest.cpp
@@ -26,6 +26,26 @@
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 {
@@ -63,7 +83,7 @@
{
created = 0;
destroyed = 0;
- char block[1024];
+ char block[64];
SkArenaAlloc arena{block};
REPORTER_ASSERT(r, *arena.make<int>(3) == 3);
@@ -113,4 +133,30 @@
}
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);
}