Reland SkWriter32 growth change with build fixes.

  - SkSWriter32 resets itself with its stack block;
  - Track the full capacity of fInternal (which may be >size after a reset).

BUG=skia:2125
R=reed@google.com, iancottrell@chromium.org, iancottrell@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/158953003

git-svn-id: http://skia.googlecode.com/svn/trunk@13412 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/core/SkTDArray.h b/include/core/SkTDArray.h
index 67254cc..804b84f 100644 (file)
--- a/include/core/SkTDArray.h
+++ b/include/core/SkTDArray.h
@@ -151,25 +151,43 @@ public:
         fCount = 0;
     }
 
+    /**
+     *  Sets the number of elements in the array.
+     *  If the array does not have space for count elements, it will increase
+     *  the storage allocated to some amount greater than that required.
+     *  It will never shrink the shrink the storage.
+     */
     void setCount(int count) {
+        //  TODO(mtklein): eliminate this method, setCountExact -> setCount
+        SkASSERT(count >= 0);
         if (count > fReserve) {
-            this->growBy(count - fCount);
-        } else {
-            fCount = count;
+            this->resizeStorageToAtLeast(count);
         }
+        fCount = count;
+    }
+
+    /**
+     *  Sets the number of elements in the array.
+     *  If the array does not have space for count elements, it will increase
+     *  the storage allocated to exactly the amount required, with no remaining
+     *  reserved space.
+     *  It will never shrink the shrink the storage.
+     */
+    void setCountExact(int count) {
+        if (count > fReserve) {
+            this->resizeStorageToExact(count);
+        }
+        fCount = count;
     }
 
     void setReserve(int reserve) {
         if (reserve > fReserve) {
-            SkASSERT(reserve > fCount);
-            int count = fCount;
-            this->growBy(reserve - fCount);
-            fCount = count;
+            this->resizeStorageToAtLeast(reserve);
         }
     }
 
     T* prepend() {
-        this->growBy(1);
+        this->adjustCount(1);
         memmove(fArray + 1, fArray, (fCount - 1) * sizeof(T));
         return fArray;
     }
@@ -183,7 +201,7 @@ public:
             SkASSERT(src == NULL || fArray == NULL ||
                     src + count <= fArray || fArray + oldCount <= src);
 
-            this->growBy(count);
+            this->adjustCount(count);
             if (src) {
                 memcpy(fArray + oldCount, src, sizeof(T) * count);
             }
@@ -204,7 +222,7 @@ public:
         SkASSERT(count);
         SkASSERT(index <= fCount);
         size_t oldCount = fCount;
-        this->growBy(count);
+        this->adjustCount(count);
         T* dst = fArray + index;
         memmove(dst + count, dst, sizeof(T) * (oldCount - index));
         if (src) {
@@ -356,20 +374,43 @@ private:
     int     fReserve;
     int     fCount;
 
-    void growBy(int extra) {
-        SkASSERT(extra);
-
-        if (fCount + extra > fReserve) {
-            int size = fCount + extra + 4;
-            size += size >> 2;
+    /**
+     *  Adjusts the number of elements in the array.
+     *  This is the same as calling setCount(count() + delta).
+     */
+    void adjustCount(int delta) {
+        this->setCount(fCount + delta);
+    }
 
-            fArray = (T*)sk_realloc_throw(fArray, size * sizeof(T));
+    /**
+     *  This resizes the storage to *exactly* count elements, growing or
+     *  shrinking the allocation as needed. It does not ASSERT anything about
+     *  the previous allocation size, or about fCount.
+     *
+     *  note: does NOT modify fCount
+     */
+    void resizeStorageToExact(int count) {
+        SkASSERT(count >= 0);
+        fArray = (T*)sk_realloc_throw(fArray, count * sizeof(T));
 #ifdef SK_DEBUG
-            fData = (ArrayT*)fArray;
+        fData = (ArrayT*)fArray;
 #endif
-            fReserve = size;
-        }
-        fCount += extra;
+        fReserve = count;
+    }
+
+    /**
+     *  Increase the storage allocation such that it can hold (fCount + extra)
+     *  elements.
+     *  It never shrinks the allocation, and it may increase the allocation by
+     *  more than is strictly required, based on a private growth heuristic.
+     *
+     *  note: does NOT modify fCount
+     */
+    void resizeStorageToAtLeast(int count) {
+        SkASSERT(count > fReserve);
+        int space = count + 4;
+        space += space>>2;
+        this->resizeStorageToExact(space);
     }
 };
 

diff --git a/include/core/SkWriter32.h b/include/core/SkWriter32.h
index df1275f..7d8d764 100644 (file)
--- a/include/core/SkWriter32.h
+++ b/include/core/SkWriter32.h
@@ -67,7 +67,7 @@ public:
         size_t offset = fUsed;
         size_t totalRequired = fUsed + size;
         if (totalRequired > fCapacity) {
-            growToAtLeast(totalRequired);
+            this->growToAtLeast(totalRequired);
         }
         fUsed = totalRequired;
         return (uint32_t*)(fData + offset);
@@ -247,7 +247,9 @@ private:
  */
 template <size_t SIZE> class SkSWriter32 : public SkWriter32 {
 public:
-    SkSWriter32() : SkWriter32(fData.fStorage, SIZE) {}
+    SkSWriter32() { this->reset(); }
+
+    void reset() {this->INHERITED::reset(fData.fStorage, SIZE); }
 
 private:
     union {
@@ -255,6 +257,8 @@ private:
         double  fDoubleAlignment;
         char    fStorage[SIZE];
     } fData;
+
+    typedef SkWriter32 INHERITED;
 };
 
 #endif

diff --git a/src/core/SkWriter32.cpp b/src/core/SkWriter32.cpp
index 1e8a10c..041e2fe 100644 (file)
--- a/src/core/SkWriter32.cpp
+++ b/src/core/SkWriter32.cpp
@@ -67,20 +67,19 @@ size_t SkWriter32::WriteStringSize(const char* str, size_t len) {
     return SkAlign4(lenBytes + len + 1);
 }
 
+const size_t kMinBufferBytes = 4096;
+
 void SkWriter32::growToAtLeast(size_t size) {
-    bool wasExternal = (fExternal != NULL) && (fData == fExternal);
+    const bool wasExternal = (fExternal != NULL) && (fData == fExternal);
+    const size_t minCapacity = kMinBufferBytes +
+        SkTMax(size, fCapacity + (fCapacity >> 1));
+
     // cause the buffer to grow
-    fInternal.setCount(size);
+    fInternal.setCountExact(minCapacity);
     fData = fInternal.begin();
+    fCapacity = fInternal.reserved();
     if (wasExternal) {
         // we were external, so copy in the data
         memcpy(fData, fExternal, fUsed);
     }
-    // Find out the size the buffer grew to, it may be more than we asked for.
-    fCapacity = fInternal.reserved();
-    // Expand the array so all reserved space is "used", we maintain the
-    // amount we have written manually outside the array
-    fInternal.setCount(fCapacity);
-    SkASSERT(fInternal.count() == (int)fCapacity);
-    SkASSERT(fInternal.reserved() == (int)fCapacity);
 }