diff --git a/src/core/SkColorShader.cpp b/src/core/SkColorShader.cpp index 69d9e46b27..5551f1e630 100644 --- a/src/core/SkColorShader.cpp +++ b/src/core/SkColorShader.cpp @@ -317,7 +317,7 @@ bool SkColor4Shader::Color4Context::onChooseBlitProcs(const SkImageInfo& info, B bool SkColorShader::onAppendStages(SkRasterPipeline* p, SkColorSpace* dst, SkFallbackAlloc* scratch) const { - auto color = scratch->make(SkPM4f_from_SkColor(fColor, dst)); + auto color = scratch->copy(SkPM4f_from_SkColor(fColor, dst)); p->append(SkRasterPipeline::move_src_dst); p->append(SkRasterPipeline::constant_color, color); if (!append_gamut_transform(p, scratch, @@ -331,7 +331,7 @@ bool SkColorShader::onAppendStages(SkRasterPipeline* p, bool SkColor4Shader::onAppendStages(SkRasterPipeline* p, SkColorSpace* dst, SkFallbackAlloc* scratch) const { - auto color = scratch->make(fColor4.premul()); + auto color = scratch->copy(fColor4.premul()); p->append(SkRasterPipeline::move_src_dst); p->append(SkRasterPipeline::constant_color, color); if (!append_gamut_transform(p, scratch, fColorSpace.get(), dst)) { diff --git a/src/core/SkFixedAlloc.cpp b/src/core/SkFixedAlloc.cpp index 133245a9cb..420cdb24cd 100644 --- a/src/core/SkFixedAlloc.cpp +++ b/src/core/SkFixedAlloc.cpp @@ -11,23 +11,37 @@ SkFixedAlloc::SkFixedAlloc(void* ptr, size_t len) : fBuffer((char*)ptr), fUsed(0), fLimit(len) {} void SkFixedAlloc::undo() { - // This function is essentially make() in reverse. - - // First, read the Footer we stamped at the end. - Footer footer; - memcpy(&footer, fBuffer + fUsed - sizeof(Footer), sizeof(Footer)); - - // That tells us where the T starts and how to destroy it. - footer.dtor(fBuffer + fUsed - sizeof(Footer) - footer.len); - - // We can reuse bytes that stored the Footer, the T, and any that we skipped for alignment. - fUsed -= sizeof(Footer) + footer.len + footer.skip; + uint32_t skip_and_size; + memcpy(&skip_and_size, fBuffer + fUsed - 4, 4); + fUsed -= skip_and_size + 4; } void SkFixedAlloc::reset() { - while (fUsed) { - this->undo(); + fUsed = 0; +} + +void* SkFixedAlloc::alloc(size_t size, size_t align) { + auto aligned = ((uintptr_t)(fBuffer+fUsed) + align-1) & ~(align-1); + size_t skip = aligned - (uintptr_t)(fBuffer+fUsed); + + if (!SkTFitsIn(skip + size) || + fUsed + skip + size + 4 > fLimit) { + return nullptr; } + + // Skip ahead until aligned. + fUsed += skip; + + // Allocate size bytes. + void* ptr = (fBuffer+fUsed); + fUsed += size; + + // Stamp a footer that we can use to clean up. + uint32_t skip_and_size = SkToU32(skip + size); + memcpy(fBuffer+fUsed, &skip_and_size, 4); + fUsed += 4; + + return ptr; } @@ -37,8 +51,7 @@ void SkFallbackAlloc::undo() { if (fHeapAllocs.empty()) { return fFixedAlloc->undo(); } - HeapAlloc alloc = fHeapAllocs.back(); - alloc.deleter(alloc.ptr); + sk_free(fHeapAllocs.back()); fHeapAllocs.pop_back(); } diff --git a/src/core/SkFixedAlloc.h b/src/core/SkFixedAlloc.h index d4e4d8f9de..0f450576d9 100644 --- a/src/core/SkFixedAlloc.h +++ b/src/core/SkFixedAlloc.h @@ -11,54 +11,51 @@ #include "SkTFitsIn.h" #include "SkTypes.h" #include +#include #include #include -// SkFixedAlloc allocates objects out of a fixed-size buffer and destroys them when destroyed. +// Before GCC 5, is_trivially_copyable had a pre-standard name. +#if defined(__GLIBCXX__) && (__GLIBCXX__ < 20150801) + namespace std { + template + using is_trivially_copyable = has_trivial_copy_constructor; + } +#endif + +// SkFixedAlloc allocates POD objects out of a fixed-size buffer. class SkFixedAlloc { public: SkFixedAlloc(void* ptr, size_t len); ~SkFixedAlloc() { this->reset(); } - // Allocates a new T in the buffer if possible. If not, returns nullptr. - template - T* make(Args&&... args) { - auto aligned = ((uintptr_t)(fBuffer+fUsed) + alignof(T) - 1) & ~(alignof(T)-1); - size_t skip = aligned - (uintptr_t)(fBuffer+fUsed); - - if (!SkTFitsIn(skip) || - !SkTFitsIn(sizeof(T)) || - fUsed + skip + sizeof(T) + sizeof(Footer) > fLimit) { - return nullptr; - } - - // Skip ahead until our buffer is aligned for T. - fUsed += skip; - - // Create the T. - auto ptr = (T*)(fBuffer+fUsed); - new (ptr) T(std::forward(args)...); - fUsed += sizeof(T); - - // Stamp a footer after the T that we can use to clean it up. - Footer footer = { [](void* ptr) { ((T*)ptr)->~T(); }, SkToU32(skip), SkToU32(sizeof(T)) }; - memcpy(fBuffer+fUsed, &footer, sizeof(Footer)); - fUsed += sizeof(Footer); - - return ptr; + // Allocates space suitable for a T. If we can't, returns nullptr. + template + void* alloc() { + static_assert(std::is_standard_layout ::value + && std::is_trivially_copyable::value, ""); + return this->alloc(sizeof(T), alignof(T)); } - // Destroys the last object allocated and frees its space in the buffer. + template + T* make(Args&&... args) { + if (auto ptr = this->alloc()) { + return new (ptr) T(std::forward(args)...); + } + return nullptr; + } + + template + T* copy(const T& src) { return this->make(src); } + + // Frees the space of the last object allocated. void undo(); - // Destroys all objects and frees all space in the buffer. + // Frees all space in the buffer. void reset(); private: - struct Footer { - void (*dtor)(void*); - uint32_t skip, len; - }; + void* alloc(size_t, size_t); char* fBuffer; size_t fUsed, fLimit; @@ -69,34 +66,34 @@ public: explicit SkFallbackAlloc(SkFixedAlloc*); ~SkFallbackAlloc() { this->reset(); } - // Allocates a new T with the SkFixedAlloc if possible. If not, uses the heap. - template - T* make(Args&&... args) { - // Once we go heap we never go back to fixed. This keeps destructor ordering sane. + template + void* alloc() { + // Once we go heap we never go back to fixed. This keeps undo() working. if (fHeapAllocs.empty()) { - if (T* ptr = fFixedAlloc->make(std::forward(args)...)) { + if (void* ptr = fFixedAlloc->alloc()) { return ptr; } } - auto ptr = new T(std::forward(args)...); - fHeapAllocs.push_back({[](void* ptr) { delete (T*)ptr; }, ptr}); + void* ptr = sk_malloc_throw(sizeof(T)); + fHeapAllocs.push_back(ptr); return ptr; } - // Destroys the last object allocated and frees any space it used in the SkFixedAlloc. + template + T* make(Args&&... args) { return new (this->alloc()) T(std::forward(args)...); } + + template + T* copy(const T& src) { return this->make(src); } + + // Frees the last object allocated, including any space it used in the SkFixedAlloc. void undo(); - // Destroys all objects and frees all space in the SkFixedAlloc. + // Frees all objects and all space in the SkFixedAlloc. void reset(); private: - struct HeapAlloc { - void (*deleter)(void*); - void* ptr; - }; - - SkFixedAlloc* fFixedAlloc; - std::vector fHeapAllocs; + SkFixedAlloc* fFixedAlloc; + std::vector fHeapAllocs; }; #endif//SkFixedAlloc_DEFINED diff --git a/src/core/SkModeColorFilter.cpp b/src/core/SkModeColorFilter.cpp index 9c76f9efed..1eb2946170 100644 --- a/src/core/SkModeColorFilter.cpp +++ b/src/core/SkModeColorFilter.cpp @@ -90,7 +90,7 @@ bool SkModeColorFilter::onAppendStages(SkRasterPipeline* p, SkColorSpace* dst, SkFallbackAlloc* scratch, bool shaderIsOpaque) const { - auto color = scratch->make(SkPM4f_from_SkColor(fColor, dst)); + auto color = scratch->copy(SkPM4f_from_SkColor(fColor, dst)); p->append(SkRasterPipeline::move_src_dst); p->append(SkRasterPipeline::constant_color, color); diff --git a/tests/FixedAllocTest.cpp b/tests/FixedAllocTest.cpp index 0a00f00935..e9bae281d5 100644 --- a/tests/FixedAllocTest.cpp +++ b/tests/FixedAllocTest.cpp @@ -10,11 +10,10 @@ namespace { - static int created, destroyed; + static int created; struct Foo { Foo(int X, float Y) : x(X), y(Y) { created++; } - ~Foo() { destroyed++; } int x; float y; @@ -38,21 +37,15 @@ DEF_TEST(FixedAlloc, r) { REPORTER_ASSERT(r, foo->x == 3); REPORTER_ASSERT(r, foo->y == 4.0f); REPORTER_ASSERT(r, created == 1); - REPORTER_ASSERT(r, destroyed == 0); Foo* bar = fa.make(8, 1.0f); REPORTER_ASSERT(r, bar); REPORTER_ASSERT(r, bar->x == 8); REPORTER_ASSERT(r, bar->y == 1.0f); REPORTER_ASSERT(r, created == 2); - REPORTER_ASSERT(r, destroyed == 0); fa.undo(); - REPORTER_ASSERT(r, created == 2); - REPORTER_ASSERT(r, destroyed == 1); } - REPORTER_ASSERT(r, created == 2); - REPORTER_ASSERT(r, destroyed == 2); { // Test alignment gurantees. @@ -62,15 +55,10 @@ DEF_TEST(FixedAlloc, r) { Foo* foo = fa.make(3, 4.0f); REPORTER_ASSERT(r, SkIsAlign4((uintptr_t)foo)); REPORTER_ASSERT(r, created == 3); - REPORTER_ASSERT(r, destroyed == 2); // Might as well test reset() while we're at it. fa.reset(); - REPORTER_ASSERT(r, created == 3); - REPORTER_ASSERT(r, destroyed == 3); } - REPORTER_ASSERT(r, created == 3); - REPORTER_ASSERT(r, destroyed == 3); } DEF_TEST(FallbackAlloc, r) { @@ -79,8 +67,8 @@ DEF_TEST(FallbackAlloc, r) { SkFixedAlloc fixed(buf, sizeof(buf)); bool fixed_failed = false; for (int i = 0; i < 32; i++) { - // (Remember, there is some overhead to each make() call.) - fixed_failed = fixed_failed || (fixed.make(i) == nullptr); + // (Remember, there is some overhead to each copy() call.) + fixed_failed = fixed_failed || (fixed.copy(i) == nullptr); } REPORTER_ASSERT(r, fixed_failed); @@ -91,7 +79,7 @@ DEF_TEST(FallbackAlloc, r) { bool fallback_failed = false; for (int i = 0; i < 32; i++) { - fallback_failed = fallback_failed || (fallback.make(i) == nullptr); + fallback_failed = fallback_failed || (fallback.copy(i) == nullptr); } REPORTER_ASSERT(r, !fallback_failed);