From 526525a37c2550ab4292a02a179ed5d5fe291919 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Tue, 15 Nov 2016 17:58:33 +0000 Subject: Revert "While we can, restrict SkFixedAlloc/SkFallbackAlloc to POD." This reverts commit 1ee70359a199f161ce38451989a706d727954cc1. Reason for revert: breaking my various rolls. Original change's description: > While we can, restrict SkFixedAlloc/SkFallbackAlloc to POD. > > This trims the overhead down to a uniform 4 bytes. > > BUG=skia: > > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4786 > > Change-Id: I92127a0c2d6c23a4a372eca39842e22195d2dc99 > Reviewed-on: https://skia-review.googlesource.com/4786 > Reviewed-by: Herb Derby > Commit-Queue: Mike Klein > TBR=mtklein@chromium.org,herb@google.com,reviews@skia.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: Iae9ef553422352a1abf95b709fccd014d468da86 Reviewed-on: https://skia-review.googlesource.com/4829 Commit-Queue: Mike Klein Reviewed-by: Mike Klein --- src/core/SkColorShader.cpp | 4 +- src/core/SkFixedAlloc.cpp | 43 +++++++------------- src/core/SkFixedAlloc.h | 89 ++++++++++++++++++++++-------------------- src/core/SkModeColorFilter.cpp | 2 +- 4 files changed, 64 insertions(+), 74 deletions(-) (limited to 'src/core') diff --git a/src/core/SkColorShader.cpp b/src/core/SkColorShader.cpp index 5551f1e630..69d9e46b27 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->copy(SkPM4f_from_SkColor(fColor, dst)); + auto color = scratch->make(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->copy(fColor4.premul()); + auto color = scratch->make(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 420cdb24cd..133245a9cb 100644 --- a/src/core/SkFixedAlloc.cpp +++ b/src/core/SkFixedAlloc.cpp @@ -11,37 +11,23 @@ SkFixedAlloc::SkFixedAlloc(void* ptr, size_t len) : fBuffer((char*)ptr), fUsed(0), fLimit(len) {} void SkFixedAlloc::undo() { - uint32_t skip_and_size; - memcpy(&skip_and_size, fBuffer + fUsed - 4, 4); - fUsed -= skip_and_size + 4; -} - -void SkFixedAlloc::reset() { - 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); + // This function is essentially make() in reverse. - if (!SkTFitsIn(skip + size) || - fUsed + skip + size + 4 > fLimit) { - return nullptr; - } + // First, read the Footer we stamped at the end. + Footer footer; + memcpy(&footer, fBuffer + fUsed - sizeof(Footer), sizeof(Footer)); - // Skip ahead until aligned. - fUsed += skip; + // That tells us where the T starts and how to destroy it. + footer.dtor(fBuffer + fUsed - sizeof(Footer) - footer.len); - // 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; + // We can reuse bytes that stored the Footer, the T, and any that we skipped for alignment. + fUsed -= sizeof(Footer) + footer.len + footer.skip; +} - return ptr; +void SkFixedAlloc::reset() { + while (fUsed) { + this->undo(); + } } @@ -51,7 +37,8 @@ void SkFallbackAlloc::undo() { if (fHeapAllocs.empty()) { return fFixedAlloc->undo(); } - sk_free(fHeapAllocs.back()); + HeapAlloc alloc = fHeapAllocs.back(); + alloc.deleter(alloc.ptr); fHeapAllocs.pop_back(); } diff --git a/src/core/SkFixedAlloc.h b/src/core/SkFixedAlloc.h index 0f450576d9..d4e4d8f9de 100644 --- a/src/core/SkFixedAlloc.h +++ b/src/core/SkFixedAlloc.h @@ -11,51 +11,54 @@ #include "SkTFitsIn.h" #include "SkTypes.h" #include -#include #include #include -// 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. +// SkFixedAlloc allocates objects out of a fixed-size buffer and destroys them when destroyed. class SkFixedAlloc { public: SkFixedAlloc(void* ptr, size_t len); ~SkFixedAlloc() { this->reset(); } - // 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)); - } - + // Allocates a new T in the buffer if possible. If not, returns nullptr. template T* make(Args&&... args) { - if (auto ptr = this->alloc()) { - return new (ptr) T(std::forward(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; } - return nullptr; - } - template - T* copy(const T& src) { return this->make(src); } + // 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); - // Frees the space of the last object allocated. + // 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; + } + + // Destroys the last object allocated and frees its space in the buffer. void undo(); - // Frees all space in the buffer. + // Destroys all objects and frees all space in the buffer. void reset(); private: - void* alloc(size_t, size_t); + struct Footer { + void (*dtor)(void*); + uint32_t skip, len; + }; char* fBuffer; size_t fUsed, fLimit; @@ -66,34 +69,34 @@ public: explicit SkFallbackAlloc(SkFixedAlloc*); ~SkFallbackAlloc() { this->reset(); } - template - void* alloc() { - // Once we go heap we never go back to fixed. This keeps undo() working. + // 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. if (fHeapAllocs.empty()) { - if (void* ptr = fFixedAlloc->alloc()) { + if (T* ptr = fFixedAlloc->make(std::forward(args)...)) { return ptr; } } - void* ptr = sk_malloc_throw(sizeof(T)); - fHeapAllocs.push_back(ptr); + auto ptr = new T(std::forward(args)...); + fHeapAllocs.push_back({[](void* ptr) { delete (T*)ptr; }, ptr}); return ptr; } - 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. + // Destroys the last object allocated and frees any space it used in the SkFixedAlloc. void undo(); - // Frees all objects and all space in the SkFixedAlloc. + // Destroys all objects and frees all space in the SkFixedAlloc. void reset(); private: - SkFixedAlloc* fFixedAlloc; - std::vector fHeapAllocs; + struct HeapAlloc { + void (*deleter)(void*); + void* ptr; + }; + + SkFixedAlloc* fFixedAlloc; + std::vector fHeapAllocs; }; #endif//SkFixedAlloc_DEFINED diff --git a/src/core/SkModeColorFilter.cpp b/src/core/SkModeColorFilter.cpp index 1eb2946170..9c76f9efed 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->copy(SkPM4f_from_SkColor(fColor, dst)); + auto color = scratch->make(SkPM4f_from_SkColor(fColor, dst)); p->append(SkRasterPipeline::move_src_dst); p->append(SkRasterPipeline::constant_color, color); -- cgit v1.2.3