speedup dynamicwstream

- move bytesWritten calculation to query the tail, allowing write() to be faster since it doesn't have to update anything extra per-write.
- enforce that all blocks are multiple-of-4 bytes big
- update the minimum block size to 4K

Before: 30ms
After:  23ms for non-4-bytes writes
        13ms for 4-bytes writes

BUG=skia:

Change-Id: Id06ecad3b9fe426747e02accf1393595e3356ce3
Reviewed-on: https://skia-review.googlesource.com/6087
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>

3 files changed, 121 insertions, 61 deletions

diff --git a/bench/StreamBench.cpp b/bench/StreamBench.cpp
index 0650a999bb..e89b207ac5 100644
--- a/bench/StreamBench.cpp
+++ b/bench/StreamBench.cpp
@@ -9,10 +9,11 @@
 #include "SkStream.h"
 
 class StreamBench : public Benchmark {
-    SkString fName;
+    SkString    fName;
+    const bool  fTestWrite4;
 public:
-    StreamBench()  {
-        fName.printf("wstream");
+    StreamBench(bool testWrite4) : fTestWrite4(testWrite4) {
+        fName.printf("wstream_%d", testWrite4);
     }
 
     bool isSuitableFor(Backend backend) override {
@@ -23,11 +24,18 @@ protected:
     const char* onGetName() override { return fName.c_str(); }
 
     void onDraw(int loops, SkCanvas* canvas) override {
+        const char t3[] = { 1, 2, 3 };
+        const char t5[] = { 1, 2, 3, 4, 5 };
         for (int i = 0; i < loops*100; ++i) {
             SkDynamicMemoryWStream stream;
-            for (int j = 0; j < 100000; ++j) {
-                stream.write32(j);
-                stream.write32(j+j);
+            for (int j = 0; j < 10000; ++j) {
+                if (fTestWrite4) {
+                    stream.write32(j);
+                    stream.write32(j+j);
+                } else {
+                    stream.write(t3, 3);
+                    stream.write(t5, 5);
+                }
             }
         }
     }
@@ -38,4 +46,5 @@ private:
 
 ///////////////////////////////////////////////////////////////////////////////
 
-DEF_BENCH(return new StreamBench;)
+DEF_BENCH(return new StreamBench(false);)
+DEF_BENCH(return new StreamBench(true);)
diff --git a/include/core/SkStream.h b/include/core/SkStream.h
index 089a4ca6f9..e6df9eabde 100644
--- a/include/core/SkStream.h
+++ b/include/core/SkStream.h
@@ -194,11 +194,19 @@ public:
 
     // helpers
 
-    bool    write8(U8CPU);
-    bool    write16(U16CPU);
-    bool    write32(uint32_t);
+    bool write8(U8CPU value)   {
+        uint8_t v = SkToU8(value);
+        return this->write(&v, 1);
+    }
+    bool write16(U16CPU value) {
+        uint16_t v = SkToU16(value);
+        return this->write(&v, 2);
+    }
+    bool write32(uint32_t v) {
+        return this->write(&v, 4);
+    }
 
-    bool    writeText(const char text[]) {
+    bool writeText(const char text[]) {
         SkASSERT(text);
         return this->write(text, strlen(text));
     }
@@ -376,10 +384,11 @@ public:
     virtual ~SkDynamicMemoryWStream();
 
     bool write(const void* buffer, size_t size) override;
-    size_t bytesWritten() const override { return fBytesWritten; }
-
+    size_t bytesWritten() const override;
     bool read(void* buffer, size_t offset, size_t size);
-    size_t getOffset() const { return fBytesWritten; }
+
+    // Why do we have this as a separate method???
+    size_t getOffset() const { return this->bytesWritten(); }
 
     // copy what has been written to the stream into dst
     void copyTo(void* dst) const;
@@ -398,7 +407,13 @@ private:
     struct Block;
     Block*  fHead;
     Block*  fTail;
-    size_t  fBytesWritten;
+    size_t  fBytesWrittenBeforeTail;
+
+#ifdef SK_DEBUG
+    void validate() const;
+#else
+    void validate() const {}
+#endif
 
     // For access to the Block type.
     friend class SkBlockMemoryStream;
diff --git a/src/core/SkStream.cpp b/src/core/SkStream.cpp
index 20afe2b701..cf473e76c4 100644
--- a/src/core/SkStream.cpp
+++ b/src/core/SkStream.cpp
@@ -107,20 +107,6 @@ bool SkWStream::writeScalarAsText(SkScalar value)
     return this->write(buffer, stop - buffer);
 }
 
-bool SkWStream::write8(U8CPU value) {
-    uint8_t v = SkToU8(value);
-    return this->write(&v, 1);
-}
-
-bool SkWStream::write16(U16CPU value) {
-    uint16_t v = SkToU16(value);
-    return this->write(&v, 2);
-}
-
-bool SkWStream::write32(uint32_t value) {
-    return this->write(&value, 4);
-}
-
 bool SkWStream::writeScalar(SkScalar value) {
     return this->write(&value, sizeof(value));
 }
@@ -467,7 +453,15 @@ bool SkMemoryWStream::write(const void* buffer, size_t size) {
 
 ////////////////////////////////////////////////////////////////////////
 
-#define SkDynamicMemoryWStream_MinBlockSize   256
+static inline void sk_memcpy_4bytes(void* dst, const void* src, size_t size) {
+    if (size == 4) {
+        memcpy(dst, src, 4);
+    } else {
+        memcpy(dst, src, size);
+    }
+}
+
+#define SkDynamicMemoryWStream_MinBlockSize   4096
 
 struct SkDynamicMemoryWStream::Block {
     Block*  fNext;
@@ -479,29 +473,26 @@ struct SkDynamicMemoryWStream::Block {
     size_t  avail() const { return fStop - fCurr; }
     size_t  written() const { return fCurr - this->start(); }
 
-    void init(size_t size)
-    {
+    void init(size_t size) {
         fNext = nullptr;
         fCurr = this->start();
         fStop = this->start() + size;
     }
 
-    const void* append(const void* data, size_t size)
-    {
+    const void* append(const void* data, size_t size) {
         SkASSERT((size_t)(fStop - fCurr) >= size);
-        memcpy(fCurr, data, size);
+        sk_memcpy_4bytes(fCurr, data, size);
         fCurr += size;
         return (const void*)((const char*)data + size);
     }
 };
 
 SkDynamicMemoryWStream::SkDynamicMemoryWStream()
-    : fHead(nullptr), fTail(nullptr), fBytesWritten(0)
+    : fHead(nullptr), fTail(nullptr), fBytesWrittenBeforeTail(0)
 {}
 
-SkDynamicMemoryWStream::~SkDynamicMemoryWStream()
-{
-    reset();
+SkDynamicMemoryWStream::~SkDynamicMemoryWStream() {
+    this->reset();
 }
 
 void SkDynamicMemoryWStream::reset() {
@@ -512,25 +503,39 @@ void SkDynamicMemoryWStream::reset() {
         block = next;
     }
     fHead = fTail = nullptr;
-    fBytesWritten = 0;
+    fBytesWrittenBeforeTail = 0;
+}
+
+size_t SkDynamicMemoryWStream::bytesWritten() const {
+    this->validate();
+
+    if (fTail) {
+        return fBytesWrittenBeforeTail + fTail->written();
+    }
+    return 0;
 }
 
 bool SkDynamicMemoryWStream::write(const void* buffer, size_t count) {
     if (count > 0) {
-        fBytesWritten += count;
-
         size_t  size;
 
-        if (fTail != nullptr && fTail->avail() > 0) {
-            size = SkTMin(fTail->avail(), count);
-            buffer = fTail->append(buffer, size);
-            SkASSERT(count >= size);
-            count -= size;
-            if (count == 0)
-                return true;
+        if (fTail) {
+            if (fTail->avail() > 0) {
+                size = SkTMin(fTail->avail(), count);
+                buffer = fTail->append(buffer, size);
+                SkASSERT(count >= size);
+                count -= size;
+                if (count == 0) {
+                    return true;
+                }
+            }
+            // If we get here, we've just exhausted fTail, so update our tracker
+            fBytesWrittenBeforeTail += fTail->written();
         }
 
-        size = SkTMax<size_t>(count, SkDynamicMemoryWStream_MinBlockSize);
+        size = SkTMax<size_t>(count, SkDynamicMemoryWStream_MinBlockSize - sizeof(Block));
+        size = SkAlign4(size);  // ensure we're always a multiple of 4 (see padToAlign4())
+
         Block* block = (Block*)sk_malloc_throw(sizeof(Block) + size);
         block->init(size);
         block->append(buffer, count);
@@ -540,14 +545,15 @@ bool SkDynamicMemoryWStream::write(const void* buffer, size_t count) {
         else
             fHead = fTail = block;
         fTail = block;
+        this->validate();
     }
     return true;
 }
 
-bool SkDynamicMemoryWStream::read(void* buffer, size_t offset, size_t count)
-{
-    if (offset + count > fBytesWritten)
+bool SkDynamicMemoryWStream::read(void* buffer, size_t offset, size_t count) {
+    if (offset + count > this->bytesWritten()) {
         return false; // test does not partially modify
+    }
     Block* block = fHead;
     while (block != nullptr) {
         size_t size = block->written();
@@ -581,14 +587,19 @@ void SkDynamicMemoryWStream::writeToStream(SkWStream* dst) const {
     }
 }
 
-void SkDynamicMemoryWStream::padToAlign4()
-{
-    // cast to remove unary-minus warning
-    int padBytes = -(int)fBytesWritten & 0x03;
-    if (padBytes == 0)
-        return;
-    int zero = 0;
-    write(&zero, padBytes);
+void SkDynamicMemoryWStream::padToAlign4() {
+    // The contract is to write zeros until the entire stream has written a multiple of 4 bytes.
+    // Our Blocks are guaranteed always be (a) full (except the tail) and (b) a multiple of 4
+    // so it is sufficient to just examine the tail (if present).
+
+    if (fTail) {
+        // cast to remove unary-minus warning
+        int padBytes = -(int)fTail->written() & 0x03;
+        if (padBytes) {
+            int zero = 0;
+            fTail->append(&zero, padBytes);
+        }
+    }
 }
 
 sk_sp<SkData> SkDynamicMemoryWStream::detachAsData() {
@@ -603,6 +614,31 @@ sk_sp<SkData> SkDynamicMemoryWStream::detachAsData() {
     return data;
 }
 
+#ifdef SK_DEBUG
+void SkDynamicMemoryWStream::validate() const {
+    if (!fHead) {
+        SkASSERT(!fTail);
+        SkASSERT(fBytesWrittenBeforeTail == 0);
+        return;
+    }
+    SkASSERT(fTail);
+
+    size_t bytes = 0;
+    const Block* block = fHead;
+    while (block) {
+        if (block->fNext) {
+            SkASSERT(block->avail() == 0);
+            bytes += block->written();
+            SkASSERT(bytes == SkAlign4(bytes)); // see padToAlign4()
+        }
+        block = block->fNext;
+    }
+    SkASSERT(bytes == fBytesWrittenBeforeTail);
+}
+#endif
+
+////////////////////////////////////////////////////////////////////////////////////////////////
+
 class SkBlockMemoryRefCnt : public SkRefCnt {
 public:
     explicit SkBlockMemoryRefCnt(SkDynamicMemoryWStream::Block* head) : fHead(head) { }