SkPDF: Memory improvements for PDF Streams

src/pdf/SkPDFStream.cpp

Index: src/pdf/SkPDFStream.cpp
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index d21205cd38db12c18c0b821f9574738bc6c850e7..6894daad84679f0a102f4faaeb81414964b898d3 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/src/pdf/SkPDFStream.cpp
@@ -13,58 +13,43 @@
 #include "SkStream.h"
 #include "SkStreamPriv.h"
 
-SkPDFStream::SkPDFStream(SkStream* stream) : fState(kUnused_State) {
-    this->setData(stream);
-}
-
-SkPDFStream::SkPDFStream(SkData* data) : fState(kUnused_State) {
-    this->setData(data);
-}
-
 SkPDFStream::~SkPDFStream() {}
 
 void SkPDFStream::emitObject(SkWStream* stream,
                              const SkPDFObjNumMap& objNumMap,
                              const SkPDFSubstituteMap& substitutes) {
-    if (fState == kUnused_State) {
-        fState = kNoCompression_State;
-        SkDynamicMemoryWStream compressedData;
-        SkDeflateWStream deflateWStream(&compressedData);
-        SkAssertResult(SkStreamCopy(&deflateWStream, fDataStream.get()));
-        deflateWStream.finalize();
-        SkAssertResult(fDataStream->rewind());
-        if (compressedData.getOffset() < this->dataSize()) {
-            SkAutoTDelete<SkStream> compressed(
-                    compressedData.detachAsStream());
-            this->setData(compressed.get());
-            this->insertName("Filter", "FlateDecode");
-        }
-        fState = kCompressed_State;
-        this->insertInt("Length", this->dataSize());
-    }
+    SkASSERT(fCompressedData);
     this->INHERITED::emitObject(stream, objNumMap, substitutes);
+    // Note: emitObject isn't marked const, but could be in the future
+    SkAutoTDelete<SkStreamRewindable> dup(fCompressedData->duplicate());

[tomhudson, 2015/08/19 19:27:51]

This looks expensive, but I assume the writing is non-const? Maybe the duplicate
is copy-on-write?

[hal.canary, 2015/08/19 19:34:20]

The duplicate just refs the underlying SkBlockMemoryRefCnt.  See
SkBlockMemoryStream::duplicate().  This is cheap.

+    SkASSERT(dup);
+    SkASSERT(dup->hasLength());
     stream->writeText(" stream\n");
-    stream->writeStream(fDataStream.get(), fDataStream->getLength());
-    SkAssertResult(fDataStream->rewind());
+    stream->writeStream(dup.get(), dup->getLength());
     stream->writeText("\nendstream");
 }
 
-SkPDFStream::SkPDFStream() : fState(kUnused_State) {}
-
-void SkPDFStream::setData(SkData* data) {
-    // FIXME: Don't swap if the data is the same.
-    fDataStream.reset(SkNEW_ARGS(SkMemoryStream, (data)));
-}
-
 void SkPDFStream::setData(SkStream* stream) {
+    SkASSERT(!fCompressedData);  // Only call this function once.
     SkASSERT(stream);
-    // Code assumes that the stream starts at the beginning and is rewindable.
-    // SkStreamRewindableFromSkStream will try stream->duplicate().
-    fDataStream.reset(SkStreamRewindableFromSkStream(stream));
-    SkASSERT(fDataStream.get());
-}
-
-size_t SkPDFStream::dataSize() const {
-    SkASSERT(fDataStream->hasLength());
-    return fDataStream->getLength();
+    // Code assumes that the stream starts at the beginning.
+
+    SkDynamicMemoryWStream compressedData;
+    SkDeflateWStream deflateWStream(&compressedData);
+    SkStreamCopy(&deflateWStream, stream);
+    deflateWStream.finalize();
+    size_t length = compressedData.bytesWritten();
+
+    if (stream->hasLength()) {
+        SkAutoTDelete<SkStreamRewindable> dup(stream->duplicate());
+        if (dup && dup->hasLength() &&
+            dup->getLength() <= length + strlen("/Filter_/FlateDecode_")) {
+            this->insertInt("Length", dup->getLength());
+            fCompressedData.reset(dup.detach());
+            return;
+        }
+    }
+    fCompressedData.reset(compressedData.detachAsStream());
+    this->insertName("Filter", "FlateDecode");
+    this->insertInt("Length", length);
 }