From dfd5f6edf83ff6b42bf38f03b9633c65d80eb6cb Mon Sep 17 00:00:00 2001 From: mtklein Date: Thu, 13 Nov 2014 11:23:36 -0800 Subject: [PATCH] Revert of Deparameterize SkVarAlloc. (patchset #6 id:100001 of https://codereview.chromium.org/721313002/) Reason for revert: Unit test failures on 32-bit machines. test Record_Alignment: ../../tests/RecordTest.cpp:100 is_aligned(record.alloc()) Original issue's description: > Deparameterize SkVarAlloc. > > SkRecord performance is not sensitive to these values, so we can cut some > storage. This rearranges the space-remaining logic to use a count of bytes > left rather than a pointer to the end, cutting memory usage a little more. > > An SkVarAlloc used to weigh 20 or 32 bytes which now becomes 16 or 24. > > I think if I think about it a bit more I can trim off that Block* too, > getting us to 12 or 16 bytes. > > Because we now just always grow by doubling, this CL switches from storing > fSmallest to its log base 2. This has the nice effect of never having to worry > about it overflowing, and means we can probably squeeze it down into a byte > if we want, even 6 bits. > > BUG=skia: > > Committed: https://skia.googlesource.com/skia/+/bc415389855888af5a1282ca4b6bee30afa3d69d TBR=reed@google.com,mtklein@chromium.org NOTREECHECKS=true NOTRY=true BUG=skia: Review URL: https://codereview.chromium.org/718203006 --- src/core/SkRecord.h | 2 +- src/core/SkVarAlloc.cpp | 12 +++++++----- src/core/SkVarAlloc.h | 15 +++++++++------ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/core/SkRecord.h b/src/core/SkRecord.h index 6019e03406..8302db4505 100644 --- a/src/core/SkRecord.h +++ b/src/core/SkRecord.h @@ -30,7 +30,7 @@ class SkRecord : SkNoncopyable { kFirstReserveCount = 64 / sizeof(void*), }; public: - SkRecord() : fCount(0), fReserved(0) {} + SkRecord() : fAlloc(1024, 2.0f), fCount(0), fReserved(0) {} ~SkRecord() { Destroyer destroyer; diff --git a/src/core/SkVarAlloc.cpp b/src/core/SkVarAlloc.cpp index a636c54198..5c3a41c6cf 100644 --- a/src/core/SkVarAlloc.cpp +++ b/src/core/SkVarAlloc.cpp @@ -20,10 +20,11 @@ struct SkVarAlloc::Block { } }; -SkVarAlloc::SkVarAlloc() +SkVarAlloc::SkVarAlloc(size_t smallest, float growth) : fByte(NULL) - , fRemaining(0) - , fLgMinSize(0) + , fLimit(NULL) + , fSmallest(SkToUInt(smallest)) + , fGrowth(growth) , fBlock(NULL) {} SkVarAlloc::~SkVarAlloc() { @@ -38,13 +39,14 @@ SkVarAlloc::~SkVarAlloc() { void SkVarAlloc::makeSpace(size_t bytes, unsigned flags) { SkASSERT(SkIsAlignPtr(bytes)); - size_t alloc = 1<<(fLgMinSize++); + size_t alloc = fSmallest; while (alloc < bytes + sizeof(Block)) { alloc *= 2; } fBlock = Block::Alloc(fBlock, alloc, flags); fByte = fBlock->data(); - fRemaining = alloc - sizeof(Block); + fLimit = fByte + alloc - sizeof(Block); + fSmallest = SkToUInt(SkScalarTruncToInt(fSmallest * fGrowth)); #if defined(SK_BUILD_FOR_MAC) SkASSERT(alloc == malloc_good_size(alloc)); diff --git a/src/core/SkVarAlloc.h b/src/core/SkVarAlloc.h index 19b2d75e76..0a7864b37a 100644 --- a/src/core/SkVarAlloc.h +++ b/src/core/SkVarAlloc.h @@ -5,21 +5,22 @@ class SkVarAlloc : SkNoncopyable { public: - SkVarAlloc(); + // SkVarAlloc will never allocate less than smallest bytes at a time. + // When it allocates a new block, it will be at least growth times bigger than the last. + SkVarAlloc(size_t smallest, float growth); ~SkVarAlloc(); // Returns contiguous bytes aligned at least for pointers. You may pass SK_MALLOC_THROW, etc. char* alloc(size_t bytes, unsigned sk_malloc_flags) { bytes = SkAlignPtr(bytes); - if (bytes > fRemaining) { + if (fByte + bytes > fLimit) { this->makeSpace(bytes, sk_malloc_flags); } - SkASSERT(bytes <= fRemaining); + SkASSERT(fByte + bytes <= fLimit); char* ptr = fByte; fByte += bytes; - fRemaining -= bytes; return ptr; } @@ -27,8 +28,10 @@ private: void makeSpace(size_t bytes, unsigned flags); char* fByte; - unsigned fRemaining; - unsigned fLgMinSize; + const char* fLimit; + + unsigned fSmallest; + const float fGrowth; struct Block; Block* fBlock;