Cleaner external buffer handling in SkWriter32

This unifies the internal and external buffer handling so that the difference only has to be noticed when growing.
Removing the branches from the common read and write cases gives a significant speedup.

BUG=skia:2125
R=tomhudson@google.com, mtklein@google.com, reed@google.com

Author: iancottrell@google.com

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

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

diff --git a/include/core/SkTDArray.h b/include/core/SkTDArray.h
index 14447217e93416626d06d820d0b2b0d58d441987..67254ccc9a88fa93fe6489ea82bccb6efe8959e6 100644 (file)
--- a/include/core/SkTDArray.h
+++ b/include/core/SkTDArray.h
@@ -100,6 +100,13 @@ public:
      */
     int count() const { return fCount; }
 
+    /**
+     *  Return the total number of elements allocated.
+     *  reserved() - count() gives you the number of elements you can add
+     *  without causing an allocation.
+     */
+    int reserved() const { return fReserve; }
+
     /**
      *  return the number of bytes in the array: count * sizeof(T)
      */

diff --git a/include/core/SkWriter32.h b/include/core/SkWriter32.h
index b193cbe7c407663f7a82a613b8a65b5e1e637a47..df1275f7b656829ad132f7b481508e42b53d99e7 100644 (file)
--- a/include/core/SkWriter32.h
+++ b/include/core/SkWriter32.h
@@ -30,12 +30,17 @@ public:
      *  first time an allocation doesn't fit.  From then it will use dynamically allocated storage.
      *  This used to be optional behavior, but pipe now relies on it.
      */
-    SkWriter32(void* external = NULL, size_t externalBytes = 0) {
+    SkWriter32(void* external = NULL, size_t externalBytes = 0)
+        : fData(0)
+        , fCapacity(0)
+        , fUsed(0)
+        , fExternal(0)
+    {
         this->reset(external, externalBytes);
     }
 
     // return the current offset (will always be a multiple of 4)
-    size_t bytesWritten() const { return fCount * 4; }
+    size_t bytesWritten() const { return fUsed; }
 
     SK_ATTR_DEPRECATED("use bytesWritten")
     size_t size() const { return this->bytesWritten(); }
@@ -43,45 +48,43 @@ public:
     void reset(void* external = NULL, size_t externalBytes = 0) {
         SkASSERT(SkIsAlign4((uintptr_t)external));
         SkASSERT(SkIsAlign4(externalBytes));
-        fExternal = (uint32_t*)external;
-        fExternalLimit = SkToInt(externalBytes/4);
-        fCount = 0;
-        fInternal.rewind();
+
+        fData = (uint8_t*)external;
+        fCapacity = externalBytes;
+        fUsed = 0;
+        fExternal = external;
     }
 
-    // If all data written is contiguous, then this returns a pointer to it, otherwise NULL.
-    // This will work if we've only written to the externally supplied block of storage, or if we've
-    // only written to our internal dynamic storage, but will fail if we have written into both.
+    // Returns the current buffer.
+    // The pointer may be invalidated by any future write calls.
     const uint32_t* contiguousArray() const {
-        if (this->externalCount()  == 0) {
-            return fInternal.begin();
-        } else if (fInternal.isEmpty()) {
-            return fExternal;
-        }
-        return NULL;
+        return (uint32_t*)fData;
     }
 
     // size MUST be multiple of 4
     uint32_t* reserve(size_t size) {
         SkASSERT(SkAlign4(size) == size);
-        const int count = SkToInt(size/4);
-
-        uint32_t* p;
-        // Once we start writing to fInternal, we never write to fExternal again.
-        // This simplifies tracking what data is where.
-        if (fInternal.isEmpty() && this->externalCount() + count <= fExternalLimit) {
-            p = fExternal + fCount;
-        } else {
-            p = fInternal.append(count);
+        size_t offset = fUsed;
+        size_t totalRequired = fUsed + size;
+        if (totalRequired > fCapacity) {
+            growToAtLeast(totalRequired);
         }
-
-        fCount += count;
-        return p;
+        fUsed = totalRequired;
+        return (uint32_t*)(fData + offset);
     }
 
     // Read or write 4 bytes at offset, which must be a multiple of 4 <= size().
-    uint32_t read32At(size_t offset) { return this->atOffset(offset); }
-    void write32At(size_t offset, uint32_t val) { this->atOffset(offset) = val; }
+    uint32_t read32At(size_t offset) {
+        SkASSERT(SkAlign4(offset) == offset);
+        SkASSERT(offset < fUsed);
+        return *(uint32_t*)(fData + offset);
+    }
+
+    void write32At(size_t offset, uint32_t val) {
+        SkASSERT(SkAlign4(offset) == offset);
+        SkASSERT(offset < fUsed);
+        *(uint32_t*)(fData + offset) = val;
+    }
 
     bool writeBool(bool value) {
         this->write32(value);
@@ -157,8 +160,6 @@ public:
      */
     void write(const void* values, size_t size) {
         SkASSERT(SkAlign4(size) == size);
-        // TODO: If we're going to spill from fExternal to fInternal, we might want to fill
-        // fExternal as much as possible before writing to fInternal.
         memcpy(this->reserve(size), values, size);
     }
 
@@ -209,26 +210,17 @@ public:
      */
     void rewindToOffset(size_t offset) {
         SkASSERT(SkAlign4(offset) == offset);
-        const int count = SkToInt(offset/4);
-        if (count < this->externalCount()) {
-            fInternal.setCount(0);
-        } else {
-            fInternal.setCount(count - this->externalCount());
-        }
-        fCount = count;
+        SkASSERT(offset <= bytesWritten());
+        fUsed = offset;
     }
 
     // copy into a single buffer (allocated by caller). Must be at least size()
     void flatten(void* dst) const {
-        const size_t externalBytes = this->externalCount()*4;
-        memcpy(dst, fExternal, externalBytes);
-        dst = (uint8_t*)dst + externalBytes;
-        memcpy(dst, fInternal.begin(), fInternal.bytes());
+        memcpy(dst, fData, fUsed);
     }
 
     bool writeToStream(SkWStream* stream) const {
-        return stream->write(fExternal, this->externalCount()*4)
-            && stream->write(fInternal.begin(), fInternal.bytes());
+        return stream->write(fData, fUsed);
     }
 
     // read from the stream, and write up to length bytes. Return the actual
@@ -238,25 +230,13 @@ public:
     }
 
 private:
-    uint32_t& atOffset(size_t offset) {
-        SkASSERT(SkAlign4(offset) == offset);
-        const int count = SkToInt(offset/4);
-        SkASSERT(count < fCount);
-
-        if (count < this->externalCount()) {
-            return fExternal[count];
-        }
-        return fInternal[count - this->externalCount()];
-    }
-
-
-    // Number of uint32_t written into fExternal.  <= fExternalLimit.
-    int externalCount() const { return fCount - fInternal.count(); }
+    void growToAtLeast(size_t size);
 
-    int fCount;                     // Total number of uint32_t written.
-    int fExternalLimit;             // Number of uint32_t we can write to fExternal.
-    uint32_t* fExternal;            // Unmanaged memory block.
-    SkTDArray<uint32_t> fInternal;  // Managed memory block.
+    uint8_t* fData;                // Points to either fInternal or fExternal.
+    size_t fCapacity;              // Number of bytes we can write to fData.
+    size_t fUsed;                  // Number of bytes written.
+    void* fExternal;               // Unmanaged memory block.
+    SkTDArray<uint8_t> fInternal;  // Managed memory block.
 };
 
 /**

diff --git a/src/core/SkWriter32.cpp b/src/core/SkWriter32.cpp
index 5e89ed655b70e5d83da4df2588704689898b52e6..7285459c3d1e5767fff762d0bb38a4a6f848a2a0 100644 (file)
--- a/src/core/SkWriter32.cpp
+++ b/src/core/SkWriter32.cpp
@@ -66,3 +66,21 @@ size_t SkWriter32::WriteStringSize(const char* str, size_t len) {
     // add 1 since we also write a terminating 0
     return SkAlign4(lenBytes + len + 1);
 }
+
+void SkWriter32::growToAtLeast(size_t size) {
+    bool wasExternal = (fExternal != NULL) && (fData == fExternal);
+    // cause the buffer to grow
+    fInternal.setCount(size);
+    fData = fInternal.begin();
+    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() == fCapacity);
+    SkASSERT(fInternal.reserved() == fCapacity);
+}