SkData to SkStreamAsset to avoid unneeded copying

dm/DMPDFTask.cpp

Index: dm/DMPDFTask.cpp
diff --git a/dm/DMPDFTask.cpp b/dm/DMPDFTask.cpp
index 5fee403a5b9b5247fc18a58059c80848f1266c11..48919ba40b09545267ab53a3708acecaa21b33e8 100644
--- a/dm/DMPDFTask.cpp
+++ b/dm/DMPDFTask.cpp
@@ -50,11 +50,11 @@ public:
 
     SkCanvas* canvas() { return fCanvas; }
 
-    SkData* end() {
+    SkStreamAsset* end() {
         fCanvas->flush();
         fDocument->endPage();
         fDocument->close();
-        return fWriteStream.copyToData();

[mtklein, 2014/08/25 21:44:34]

Is this the copy we're avoiding?

[hal.canary, 2014/08/26 15:20:50]

Yes.  copyToData() is O(N) while detachAsStream() is O(1).

+        return fWriteStream.detachAsStream();
     }
 
 private:
@@ -66,7 +66,7 @@ private:
 }  // namespace
 
 void PDFTask::draw() {
-    SkAutoTUnref<SkData> pdfData;
+    SkAutoTUnref<SkStreamAsset> pdfData;
     bool rasterize = true;
     if (fGM.get()) {
         rasterize = 0 == (fGM->getFlags() & skiagm::GM::kSkipPDFRasterization_Flag);
@@ -83,9 +83,10 @@ void PDFTask::draw() {
 
     SkASSERT(pdfData.get());
     if (rasterize) {
-        this->spawnChild(SkNEW_ARGS(PDFRasterizeTask, (*this, pdfData.get(), fRasterize)));
+        this->spawnChild(SkNEW_ARGS(PDFRasterizeTask,
+                                    (*this, pdfData->duplicate(), fRasterize)));
     }
-    this->spawnChild(SkNEW_ARGS(WriteTask, (*this, pdfData.get(), ".pdf")));
+    this->spawnChild(SkNEW_ARGS(WriteTask, (*this, pdfData.detach(), ".pdf")));

[mtklein, 2014/08/25 21:44:33]

What's the cost of duplicate()?  It's weird one kid gets a duplicate and the
other gets the original.

[hal.canary, 2014/08/26 15:20:51]

The duplicate is cheap.  It's only a SkNEW SkBlockMemoryStream and a ref() on
the SkBlockMemoryRefCnt.
I had one object and I need two.  The simplest thing is to make one copy.  I
could make two copies and delete the original, but why?

[mtklein, 2014/08/26 15:30:54]

Just to make it a little more consistent and resilient to unrelated code
changes.  If I swapped the order of child calls or made WriteTask conditional,
things would break.  

We can certainly think through the code and determine it's safe now, but it'd be
even nicer for us to not have to think in the future and just glance to see it's
correct.

 }
 
 bool PDFTask::shouldSkip() const {