commit 5adaf8bf24e7581104c41c868188602624e8ba86
author Mike Reed <reed@google.com> Thu Dec 15 13:02:33 2016 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Dec 15 19:01:02 2016 +0000
tree 134e6d015f48fe1bbb246029494cd7cb2c741009
parent eaef5493ca51ccc722f594ed21d331d40bffd14c

remove unused dynamicwstream.snapshotAsData()

Checking to invalidate this on every write() call has a measurable cost, so removing it both simplifies the class and speeds it up.

BUG=skia:

Change-Id: Idf0baa265c9a0b5d26d82fce948c61ed9b0810b1
Reviewed-on: https://skia-review.googlesource.com/6096
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>

include/core/SkStream.h

diff --git a/include/core/SkStream.h b/include/core/SkStream.h
index d8eeb6c..089a4ca 100644
--- a/include/core/SkStream.h
+++ b/include/core/SkStream.h
@@ -385,7 +385,6 @@
     void copyTo(void* dst) const;
     void writeToStream(SkWStream* dst) const;
 
-    sk_sp<SkData> snapshotAsData() const;
     // Return the contents as SkData, and then reset the stream.
     sk_sp<SkData> detachAsData();
 
@@ -400,9 +399,6 @@
     Block*  fHead;
     Block*  fTail;
     size_t  fBytesWritten;
-    mutable sk_sp<SkData> fCopy;  // is invalidated if we write after it is created
-
-    void invalidateCopy();
 
     // For access to the Block type.
     friend class SkBlockMemoryStream;

src/core/SkStream.cpp

diff --git a/src/core/SkStream.cpp b/src/core/SkStream.cpp
index cb47437..20afe2b 100644
--- a/src/core/SkStream.cpp
+++ b/src/core/SkStream.cpp
@@ -504,12 +504,8 @@
     reset();
 }
 
-void SkDynamicMemoryWStream::reset()
-{
-    this->invalidateCopy();
-
+void SkDynamicMemoryWStream::reset() {
     Block*  block = fHead;
-
     while (block != nullptr) {
         Block*  next = block->fNext;
         sk_free(block);
@@ -519,11 +515,8 @@
     fBytesWritten = 0;
 }
 
-bool SkDynamicMemoryWStream::write(const void* buffer, size_t count)
-{
+bool SkDynamicMemoryWStream::write(const void* buffer, size_t count) {
     if (count > 0) {
-        this->invalidateCopy();
-
         fBytesWritten += count;
 
         size_t  size;
@@ -572,19 +565,13 @@
     return false;
 }
 
-void SkDynamicMemoryWStream::copyTo(void* dst) const
-{
-    if (fCopy) {
-        memcpy(dst, fCopy->data(), fBytesWritten);
-    } else {
-        Block* block = fHead;
-
-        while (block != nullptr) {
-            size_t size = block->written();
-            memcpy(dst, block->start(), size);
-            dst = (void*)((char*)dst + size);
-            block = block->fNext;
-        }
+void SkDynamicMemoryWStream::copyTo(void* dst) const {
+    Block* block = fHead;
+    while (block != nullptr) {
+        size_t size = block->written();
+        memcpy(dst, block->start(), size);
+        dst = (void*)((char*)dst + size);
+        block = block->fNext;
     }
 }
 
@@ -604,24 +591,16 @@
     write(&zero, padBytes);
 }
 
-sk_sp<SkData> SkDynamicMemoryWStream::snapshotAsData() const {
-    if (nullptr == fCopy) {
-        auto data = SkData::MakeUninitialized(fBytesWritten);
-        // be sure to call copyTo() before we assign to fCopy
-        this->copyTo(data->writable_data());
-        fCopy = std::move(data);
-    }
-    return fCopy;
-}
-
 sk_sp<SkData> SkDynamicMemoryWStream::detachAsData() {
-    sk_sp<SkData> data = this->snapshotAsData();
-    this->reset();
-    return data;
-}
+    const size_t size = this->bytesWritten();
+    if (0 == size) {
+        return SkData::MakeEmpty();
+    }
 
-void SkDynamicMemoryWStream::invalidateCopy() {
-    fCopy = nullptr;
+    sk_sp<SkData> data = SkData::MakeUninitialized(size);
+    this->copyTo(data->writable_data());
+    this->reset(); // this is the "detach" part
+    return data;
 }
 
 class SkBlockMemoryRefCnt : public SkRefCnt {
@@ -770,13 +749,8 @@
 };
 
 SkStreamAsset* SkDynamicMemoryWStream::detachAsStream() {
-    if (fCopy) {
-        SkMemoryStream* stream = new SkMemoryStream(fCopy);
-        this->reset();
-        return stream;
-    }
-    SkBlockMemoryStream* stream = new SkBlockMemoryStream(fHead, fBytesWritten);
-    fHead = 0;
+    SkBlockMemoryStream* stream = new SkBlockMemoryStream(fHead, this->bytesWritten());
+    fHead = nullptr;    // signal reset() to not free anything
     this->reset();
     return stream;
 }

tests/PDFGlyphsToUnicodeTest.cpp

diff --git a/tests/PDFGlyphsToUnicodeTest.cpp b/tests/PDFGlyphsToUnicodeTest.cpp
index b3ee2d8..a256c93 100644
--- a/tests/PDFGlyphsToUnicodeTest.cpp
+++ b/tests/PDFGlyphsToUnicodeTest.cpp
@@ -15,14 +15,19 @@
 
 static bool stream_equals(const SkDynamicMemoryWStream& stream, size_t offset,
                           const char* buffer, size_t len) {
-    sk_sp<SkData> data = stream.snapshotAsData();
-    if (offset + len > data->size()) {
-        return false;
-    }
     if (len != strlen(buffer)) {
         return false;
     }
-    return memcmp(data->bytes() + offset, buffer, len) == 0;
+
+    const size_t streamSize = stream.bytesWritten();
+
+    if (offset + len > streamSize) {
+        return false;
+    }
+
+    SkAutoTMalloc<char> data(streamSize);
+    stream.copyTo(data.get());
+    return memcmp(data.get() + offset, buffer, len) == 0;
 }
 
 DEF_TEST(SkPDF_ToUnicode, reporter) {

tests/StreamTest.cpp

diff --git a/tests/StreamTest.cpp b/tests/StreamTest.cpp
index d1fd751..b64de69 100644
--- a/tests/StreamTest.cpp
+++ b/tests/StreamTest.cpp
@@ -118,12 +118,6 @@
     REPORTER_ASSERT(reporter, ds.getOffset() == 100 * 26);
 
     {
-        sk_sp<SkData> data = ds.snapshotAsData();
-        REPORTER_ASSERT(reporter, 100 * 26 == data->size());
-        REPORTER_ASSERT(reporter, memcmp(dst, data->data(), data->size()) == 0);
-    }
-
-    {
         // Test that this works after a snapshot.
         std::unique_ptr<SkStreamAsset> stream(ds.detachAsStream());
         REPORTER_ASSERT(reporter, ds.getOffset() == 0);