Add capture snapshot as data to SkWriter32, use it to optimise record->playback.

src/core/SkWriter32.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkReader32.h"
 #include "SkString.h"
 #include "SkWriter32.h"
@@ skipping 57 matching lines @@
 
     fCapacity = 4096 + SkTMax(size, fCapacity + (fCapacity / 2));
     fInternal.realloc(fCapacity);
     fData = fInternal.get();
 
     if (wasExternal) {
         // we were external, so copy in the data
         memcpy(fData, fExternal, fUsed);
     }
 }
+
+SkData* SkWriter32::snapshotAsData() {
+    // we use size change detection to invalidate the cached data
+    if ((fSnapshot.get() != NULL) && (fSnapshot->size() != fUsed)) {

[reed1, 2014/02/14 17:31:08]

I wonder if we should consider invalidating this cache explicitly whenever we
grow our allocator, just to avoid the memory bloat of an unreachable SkData
cache.

[iancottrell, 2014/02/14 18:28:36]

Yes, I did consider that alternative.
Basically we need to addref in here before we return, and make sure we unref
sometime after we stop using it.
We could easily add the unref in the external->internal path in the growth code.

[ian_cottrell, 2014/02/17 14:29:57]

I added the snapshot drop in the growth code, but it is worth keeping this test
as well, as it will trigger even when the buffer did not grow but the space used
in it did.

+        fSnapshot.reset(NULL);
+    }
+    if (fSnapshot.get() == NULL) {
+        uint8_t* buffer = NULL;
+        if ((fExternal != NULL) && (fData == fExternal)) {
+            // We need to copy to an allocated buffer before returning.
+            buffer = (uint8_t*)sk_malloc_throw(fUsed);
+            memcpy(buffer, fData, fUsed);
+        } else {
+            buffer = fInternal.detach();
+            // prepare us to do copy on write, by pretending the data buffer
+            // is external and size limited

[mtklein, 2014/02/14 17:42:18]

Have I got this right, that we're detaching and re-reffing the internal storage,
treating it as completely full external storage in the future?  That way, the
external to internal copy-on-overflow doubles as copy-on-write?

Can we simplify things further by unifying fExternal and fSnapshot?  fExternal
becomes an SkAutoTUnref<SkData>, and we use different subclasses of SkData to
control whether or not fExternal holds a ref on the buffer?

[iancottrell, 2014/02/14 18:28:36]

Exactly, yes.
Yes, I guess I just need to use SkData::NewWithProc and hand it a release proc
that does nothing.

[iancottrell, 2014/02/27 20:16:19]

I looked at doing this, and it turns out to be awkward and gain you nothing, as
the case exists where making a snapshot causes you to have to copy the external
buffer first, which means you need to be able to tell the difference.

[mtklein, 2014/03/06 18:58:07]

Thanks for investigating.

+            fData = buffer;

[reed1, 2014/02/14 17:35:27]

What if someone does a readAt(), and the snapshot has been deleted by the
client? Won't this buffer be pointing at bad-memory?

[iancottrell, 2014/02/14 18:28:36]

no, so long as we hold a ref the buffer should be valid.

+            fCapacity = fUsed;
+            fExternal = buffer;
+        }
+        fSnapshot.reset(SkData::NewFromMalloc(buffer, fUsed));
+    }
+    return SkRef(fSnapshot.get());
+}