Change growth function for SkWriter32

include/core/SkTDArray.h

 
 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #ifndef SkTDArray_DEFINED
@@ skipping 133 matching lines @@
         } else {
             SkASSERT(fReserve == 0 && fCount == 0);
         }
     }
 
     void rewind() {
         // same as setCount(0)
         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) {
         if (count > fReserve) {
             this->growBy(count - fCount);
         } else {

[reed1, 2014/02/07 17:05:15]

This is so hard to read (for me this time at least). It appears that sometimes
we set fCount and sometimes we don't! I now know that growBy as the side-effect
of updating fCount, but that is not clear at all (to me), given that we have
this if/else, and given that setCountExact *always* sets fCount, even though it
also checks reserve.

             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.

[reed1, 2014/02/07 17:05:15]

... * this call allows the caller explicit control over the growth/allocation
strategy. calling setCount() defers to the class as to how much extra should be
allocated when the request requires a larger allocation.

+     */
+    void setCountExact(int count) {
+        if (count > fReserve) {
+            this->growTo(count);
+        }
+        fCount = count;
+    }
+
     void setReserve(int reserve) {
         if (reserve > fReserve) {
             SkASSERT(reserve > fCount);
             int count = fCount;
             this->growBy(reserve - fCount);
             fCount = count;
         }
     }
 
     T* prepend() {
@@ skipping 177 matching lines @@
     enum {
         kDebugArraySize = 16
     };
     typedef T ArrayT[kDebugArraySize];
     ArrayT* fData;
 #endif
     T*      fArray;
     int     fReserve;
     int     fCount;
 
+    /**
+     *  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.
+     */
+    void growTo(int count) {

[reed1, 2014/02/07 17:05:15]

rename to: resizeStorageTo() so we know what's going on at the call-sites.

+        fArray = (T*)sk_realloc_throw(fArray, count * sizeof(T));
+#ifdef SK_DEBUG
+        fData = (ArrayT*)fArray;
+#endif
+        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.
+     */
     void growBy(int extra) {
         SkASSERT(extra);
 
         if (fCount + extra > fReserve) {

[reed1, 2014/02/07 17:05:15]

Slightly unrelated to this CL, but I feel like we should take this opportunity
to clean-up this class if possible:

Some clients of growBy() check reserve first, and some don't. Can we unify that
pattern one way or the other? I ask partly to help clarify (to implementors)
what each help method is really doing, what it is checking/asserting on, etc.

-            int size = fCount + extra + 4;
-            size += size >> 2;
-
-            fArray = (T*)sk_realloc_throw(fArray, size * sizeof(T));
-#ifdef SK_DEBUG
-            fData = (ArrayT*)fArray;
-#endif
-            fReserve = size;
+            int count = fCount + extra + 4;
+            growTo(count + (count >> 2));
         }
         fCount += extra;
     }
 };
 
 #endif