Start from scratch on a faster SkFlatDictionary.

Start from scratch on a faster SkFlatDictionary.

This is like codereview.chromium.org/19276003, except it fits in better with the existing code, doesn't leak memory, and because it's back using SkChunkFlatController it's a little faster too, now a win across the board:

Slowdown	bench
-1.59%	desk_youtubetvbrowse.skp
-2.56%	desk_googlehome.skp
-6.40%	tabl_androidpolice.skp
-6.45%	desk_youtubetvvideo.skp
-6.91%	tabl_googlecalendar.skp
...
-29.70%	desk_yahoogames.skp
-32.17%	desk_googlespreadsheet.skp
-32.23%	mobi_wikipedia.skp
-37.16%	desk_chalkboard.skp
-41.57%	desk_pokemonwiki.skp
Overall slowdown: -22.74%

running bench [640 480] picture_record_recurring_paint_dictionary  NONRENDERING: cmsecs =   9.92
running bench [640 480] picture_record_unique_paint_dictionary  NONRENDERING: cmsecs =  22.16
running bench [640 480]  picture_record_dictionaries  NONRENDERING: cmsecs =   9.18

BUG=

Committed: http://code.google.com/p/skia/source/detail?r=10328

Committed: http://code.google.com/p/skia/source/detail?r=10336

[mtklein, 2013-07-18 18:16:07]

Sorry for yanking you guys around onto a different CL, but this version is
different enough that I think it really wants to start fresh.  Basic strategy is
the same, but I've pulled the code together a little more tightly into
SkFlatDictionary, and tweaked the contract and implementation of
SkFlatController a little to reflect the new usage pattern.

I think this version has addressed all the comments on the other CL.  Sorry if I
missed something.

[mtklein, 2013-07-19 18:03:25]

On 2013/07/18 18:16:07, mtklein wrote:
> Sorry for yanking you guys around onto a different CL, but this version is
> different enough that I think it really wants to start fresh.  Basic strategy
is
> the same, but I've pulled the code together a little more tightly into
> SkFlatDictionary, and tweaked the contract and implementation of
> SkFlatController a little to reflect the new usage pattern.
> 
> I think this version has addressed all the comments on the other CL.  Sorry if
I
> missed something.

Just FYI, I wrote up some code to automate my benchmarks and added significance
tests.  Flipped the thing around to speedups, so positive numbers are
improvement.

$ compare.sh bench_pictures --repeat 50 -r ../skp/*.skp --mode record
master vs. dictionaries
sig?  speedup  bench
       -1.77%  desk_youtubetvvideo.skp
        3.69%  desk_youtubetvbrowse.skp
  y     5.03%  desk_googlehome.skp
  y     6.43%  tabl_androidpolice.skp
  y     7.09%  tabl_frantzen.skp
  y     7.80%  desk_googleplus.skp
  y     8.37%  tabl_nofolo.skp
  y     8.51%  desk_booking.skp
  y     8.66%  tabl_deviantart.skp
  y     9.08%  desk_yahooanswers.skp
  y     9.30%  tabl_gspro.skp
  y     9.31%  desk_gmailthread.skp
  y    10.17%  tabl_cnet.skp
  y    10.41%  tabl_googlecalendar.skp
  y    10.78%  tabl_theverge.skp
  y    10.82%  tabl_mercurynews.skp
  y    10.95%  desk_youtube.skp
  y    11.28%  tabl_mlb.skp
  y    11.56%  tabl_culturalsolutions.skp
  y    11.81%  tabl_gmail.skp
  y    12.00%  tabl_slashdot.skp
  y    12.89%  desk_yahoonews.skp
  y    13.02%  tabl_ukwsj.skp
  y    13.44%  tabl_engadget.skp
  y    13.61%  desk_linkedin.skp
  y    13.62%  desk_yahoosports.skp
  y    13.67%  tabl_onlinewsj.skp
  y    14.05%  tabl_vnexpress.skp
  y    14.34%  tabl_cuteoverload.skp
  y    14.36%  tabl_transformice.skp
  y    14.85%  tabl_worldjournal.skp
  y    15.28%  desk_weather.skp
  y    15.52%  desk_espn.skp
  y    15.56%  desk_gws.skp
  y    15.69%  tabl_techmeme.skp
  y    15.78%  tabl_digg.skp
  y    16.30%  desk_amazon.skp
  y    16.63%  tabl_googleblog.skp
  y    16.74%  tabl_hsfi.skp
  y    16.83%  desk_br337.skp
  y    16.84%  desk_pinterest.skp
  y    17.40%  desk_ebay.skp
  y    18.83%  desk_sfgate.skp
  y    19.10%  tabl_gamedeksiam.skp
  y    19.17%  desk_googlespreadsheetdashed.skp
  y    19.20%  tabl_cnn.skp
  y    19.22%  desk_wordpress.skp
  y    19.67%  desk_facebook.skp
  y    20.33%  desk_baidu.skp
  y    20.34%  tabl_mozilla.skp
  y    20.76%  desk_css3gradients.skp
  y    21.08%  tabl_pravda.skp
  y    22.51%  desk_twitter.skp
  y    22.58%  tabl_nytimes.skp
  y    23.14%  desk_blogger.skp
  y    23.71%  desk_wowwiki.skp
  y    25.31%  desk_techcrunch.skp
  y    27.19%  tabl_sahadan.skp
  y    29.32%  desk_yahoogames.skp
  y    30.59%  desk_googlespreadsheet.skp
  y    32.41%  mobi_wikipedia.skp
  y    35.08%  desk_chalkboard.skp
  y    40.33%  desk_pokemonwiki.skp
Overall speedup: 22.01%

$ compare.sh bench  --match picture_record --repeat 100
master vs. dictionaries
sig?  speedup  bench
  y     7.48%  picture_record_unique_paint_dictionary
  y    11.78%  picture_record_dictionaries
  y    37.61%  picture_record_recurring_paint_dictionary
Overall speedup: 17.88%

[tomhudson, 2013-07-22 16:36:02]

LGTM modulo nits. Speedup numbers look great.

https://codereview.chromium.org/19564007/diff/2001/include/core/SkWriter32.h
File include/core/SkWriter32.h (right):

https://codereview.chromium.org/19564007/diff/2001/include/core/SkWriter32.h#...
include/core/SkWriter32.h:49: // DEPRECATED: use byetsWritten instead
Tangent: in SkFlatDictionary, byets write you!

https://codereview.chromium.org/19564007/diff/2001/include/core/SkWriter32.h#...
include/core/SkWriter32.h:59: void reset(void* storage, size_t size);
Thanks for moving these together.

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h#n...
src/core/SkPictureFlat.h:180: * call unalloc(), in which case it may be freed
early.
Nit: While we're fixing up this comment, the function isn't really 'Provid[ing]'
memory. Do we have a better verb?

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h#n...
src/core/SkPictureFlat.h:573: // Layout: [ SkFlatData header, 20 bytes ] [ data
... ] [ sentinel, 4 bytes]
Nit: Do we need to worry (or assert?) about the alignment of the sentinel, given
that we're treating the data as uint8?

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h#n...
src/core/SkPictureFlat.h:598: lazyWriteBufferInit();
this->lazyWriteBufferInit();

[mtklein, 2013-07-22 17:43:55]

https://codereview.chromium.org/19564007/diff/2001/include/core/SkWriter32.h
File include/core/SkWriter32.h (right):

https://codereview.chromium.org/19564007/diff/2001/include/core/SkWriter32.h#...
include/core/SkWriter32.h:49: // DEPRECATED: use byetsWritten instead
On 2013/07/22 16:36:02, tomhudson wrote:
> Tangent: in SkFlatDictionary, byets write you!

:)  Fxied

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h#n...
src/core/SkPictureFlat.h:180: * call unalloc(), in which case it may be freed
early.
On 2013/07/22 16:36:02, tomhudson wrote:
> Nit: While we're fixing up this comment, the function isn't really
'Provid[ing]'
> memory. Do we have a better verb?

How about a nice, neutral "Return"?

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h#n...
src/core/SkPictureFlat.h:573: // Layout: [ SkFlatData header, 20 bytes ] [ data
... ] [ sentinel, 4 bytes]
On 2013/07/22 16:36:02, tomhudson wrote:
> Nit: Do we need to worry (or assert?) about the alignment of the sentinel,
given
> that we're treating the data as uint8?
Oh, good call.  Using SkWriter32 guarantees that dataSize will be 4-byte
aligned, which is indeed something we're relying on to align the sentinel. 
setSentinel does this same assert itself, so we're covered already, but I think
it probably helps comprehension to document the alignment and double the assert
here.

https://codereview.chromium.org/19564007/diff/2001/src/core/SkPictureFlat.h#n...
src/core/SkPictureFlat.h:598: lazyWriteBufferInit();
On 2013/07/22 16:36:02, tomhudson wrote:
> this->lazyWriteBufferInit();

Done.

[reed1, 2013-07-22 18:07:48]

unittest for new Write32 functionality?

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.cpp
File src/core/SkPictureFlat.cpp (right):

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.cp...
src/core/SkPictureFlat.cpp:97: void SkFlatData::init(int index, int32_t size) {
Either a bigger name, or perhaps a comment saying "I don't touch my contents, in
case they've already been written"

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:536: const SkFlatData& temp =
this->getTemporary(element, fNextIndex);
scratch

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:555: SkFlatData* durable = this->getDurable();
Need some clearer naming or something to describe what's happening... I think

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:587: if (fWriteBufferReady) return;
LW nit {}

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:604: const size_t size = fWriteBuffer.size();
bytesWritten() ?

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:610:
fWriteBuffer.writeToMemory(this->dataStart(bigger));
can SkFlatData have a dataStart() method, so that this isn't acting as a
proxy-API for this embedded object?

[mtklein, 2013-07-22 18:54:48]

PTAL

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.cpp
File src/core/SkPictureFlat.cpp (right):

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.cp...
src/core/SkPictureFlat.cpp:97: void SkFlatData::init(int index, int32_t size) {
On 2013/07/22 18:07:49, reed1 wrote:
> Either a bigger name, or perhaps a comment saying "I don't touch my contents,
in
> case they've already been written"

Did both.

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:536: const SkFlatData& temp =
this->getTemporary(element, fNextIndex);
On 2013/07/22 18:07:49, reed1 wrote:
> scratch

Done.

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:555: SkFlatData* durable = this->getDurable();
On 2013/07/22 18:07:49, reed1 wrote:
> Need some clearer naming or something to describe what's happening... I think

How's this?

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:587: if (fWriteBufferReady) return;
On 2013/07/22 18:07:49, reed1 wrote:
> LW nit {}

Done.

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:604: const size_t size = fWriteBuffer.size();
On 2013/07/22 18:07:49, reed1 wrote:
> bytesWritten() ?

Done.

https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:610:
fWriteBuffer.writeToMemory(this->dataStart(bigger));
On 2013/07/22 18:07:49, reed1 wrote:
> can SkFlatData have a dataStart() method, so that this isn't acting as a
> proxy-API for this embedded object?

Done.

[reed1, 2013-07-22 19:00:49]

good from my side lgtm

[mtklein, 2013-07-22 19:03:02]

On 2013/07/22 18:54:48, mtklein wrote:
> PTAL
> 
> https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.cpp
> File src/core/SkPictureFlat.cpp (right):
> 
>
https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.cp...
> src/core/SkPictureFlat.cpp:97: void SkFlatData::init(int index, int32_t size)
{
> On 2013/07/22 18:07:49, reed1 wrote:
> > Either a bigger name, or perhaps a comment saying "I don't touch my
contents,
> in
> > case they've already been written"
> 
> Did both.
> 
> https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h
> File src/core/SkPictureFlat.h (right):
> 
>
https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
> src/core/SkPictureFlat.h:536: const SkFlatData& temp =
> this->getTemporary(element, fNextIndex);
> On 2013/07/22 18:07:49, reed1 wrote:
> > scratch
> 
> Done.
> 
>
https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
> src/core/SkPictureFlat.h:555: SkFlatData* durable = this->getDurable();
> On 2013/07/22 18:07:49, reed1 wrote:
> > Need some clearer naming or something to describe what's happening... I
think
> 
> How's this?
> 
>
https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
> src/core/SkPictureFlat.h:587: if (fWriteBufferReady) return;
> On 2013/07/22 18:07:49, reed1 wrote:
> > LW nit {}
> 
> Done.
> 
>
https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
> src/core/SkPictureFlat.h:604: const size_t size = fWriteBuffer.size();
> On 2013/07/22 18:07:49, reed1 wrote:
> > bytesWritten() ?
> 
> Done.
> 
>
https://codereview.chromium.org/19564007/diff/10001/src/core/SkPictureFlat.h#...
> src/core/SkPictureFlat.h:610:
> fWriteBuffer.writeToMemory(this->dataStart(bigger));
> On 2013/07/22 18:07:49, reed1 wrote:
> > can SkFlatData have a dataStart() method, so that this isn't acting as a
> > proxy-API for this embedded object?
> 
> Done.

OK, going to kick off the queue to submit this, with the promise to follow up
quickly on anything that may crop up later.

[tomhudson, 2013-07-23 09:41:08]

> OK, going to kick off the queue to submit this, with the promise to follow up
> quickly on anything that may crop up later.

I'd say "Ship it!", but it looks like you had a GM failure, and that in turn
suggests we might need more unit tests? :(

[mtklein, 2013-07-23 21:56:11]

On 2013/07/23 09:41:08, tomhudson wrote:
> > OK, going to kick off the queue to submit this, with the promise to follow
up
> > quickly on anything that may crop up later.
> 
> I'd say "Ship it!", but it looks like you had a GM failure, and that in turn
> suggests we might need more unit tests? :(

So, I've just uploaded one (final?) patch set and I'd appreciate it if you could
take another look.  This patch set changes how we allocate memory for the
scratch SkFlatData to be less abusive on the SkFlatController.  Upshot is that
when SkFlatController is a SkChunkFlatController, this CL used to allocate more
than 2x the bytes it really needed from the SkFlatController, but now that's
fixed.

Recall that an SkFlatData looks like [ 20 byte header] [ data ... ] [4 byte
sentinel].  Now, your average paint needs about 100 bytes to store itself, so we
should see an average of about 124 bytes needed to store a paint, an overhead of
25%.

Until this patch set this code was allocating all its scratch memory from the
controller, which isn't great if your controller can't reuse unallocated blocks.
 SkChunkFlatController can only recycle its last returned pointer.  Anything
else unalloc'd will actually stick around until it dies.  So, one (relatively
minor) place we were using extra memory was reallocating scratch when it was too
small: we dutifully called unalloc on our old buffer but that was never the last
buffer allocated (the new bigger one was).  If say, it started at 64 bytes, then
grew to 128 bytes, all 196 bytes would actually stick around until the
controller was free'd (that is, until recording is done).  This wasn't a huge
deal because we only grew upward, so pretty quickly we'd hit a scratch size
that's big enough and this waste stops.

But that upward growth led to the real problem.  When we wanted to promote our
scratch SkFlatData to something that'd stick around, we'd just release the
pointer we had and allocate a new one of the same size.   Problem is, our
scratch SkFlatData had grown to the maximum size it'd seen, and when we detach
it, we're likely giving away a perfectly fine SkFlatData and a whole bunch of
extra unused bytes.  Essentially, the cost to store any given paint became the
maximum size paint we'd seen to date.  Most paints are not the maximum size
paint, so this wasted a lot of space.

These two problems (never freeing scratch and always allocating for maximum
size) lead to an overhead of 188%.  That is, it took almost 3x as much space to
store the paints as they naturally occupy (compare 2.88x broken vs. 1.25x fixed
vs. 1x ideal).

So, here's how I fixed it:
   - scratch SkFlatData are allocated with sk_malloc and sk_free, not the
controller;
   - detachScratch() now allocs a snugly-sized SkFlatData and then we memcpy the
scratch onto it.
This way the controller only ever allocates exactly as much space as is needed,
and we never even try to unallocate SkFlatData that come out of the controller.

To get this back up to speed I also had to add back the size hints,
preallocating 512 bytes for scratch paints and 36 bytes for scratch matrices. 
These guesses mean that, running over my set of skps, we never have to
reallocate the scratch buffer.  The extra memcpy we do in detachScratch()
doesn't seem to be having a major effect on overall performance.  Remember the
whole point of this CL was to eliminate as many copies as possible in the common
case where we don't call detachScratch().  It's rare enough to call
detachScratch() that that memcpy doesn't really hurt us.

New bench_pictures and bench numbers look reassuringly similar to the old ones.

~/skia/trunk > compare.sh bench_pictures -r ../skp/*.skp --mode record --repeat
50
master vs. dictionaries
sig?  speedup  bench
        1.79%  desk_youtubetvbrowse.skp
        2.56%  desk_youtubetvvideo.skp
  y     3.12%  tabl_gspro.skp
  y     6.57%  tabl_frantzen.skp
  y     6.91%  tabl_cnet.skp
  y     7.97%  desk_googlehome.skp
  y     8.22%  tabl_androidpolice.skp
  y     8.80%  tabl_googlecalendar.skp
  y     8.95%  tabl_deviantart.skp
  y     9.49%  desk_weather.skp
  y     9.64%  desk_gmailthread.skp
  y    10.80%  desk_yahoonews.skp
  y    10.86%  tabl_culturalsolutions.skp
  y    11.05%  tabl_mercurynews.skp
  y    11.08%  desk_youtube.skp
  y    11.14%  tabl_cuteoverload.skp
  y    11.92%  desk_yahooanswers.skp
  y    12.12%  tabl_nofolo.skp
  y    12.37%  desk_yahoosports.skp
  y    12.46%  tabl_mlb.skp
  y    12.60%  desk_googleplus.skp
  y    13.48%  desk_wordpress.skp
  y    13.75%  tabl_engadget.skp
  y    13.85%  desk_amazon.skp
  y    14.01%  tabl_gmail.skp
  y    14.38%  tabl_slashdot.skp
  y    14.45%  desk_gws.skp
  y    14.97%  desk_sfgate.skp
  y    15.20%  desk_espn.skp
  y    15.31%  tabl_onlinewsj.skp
  y    15.40%  tabl_digg.skp
  y    15.46%  desk_pinterest.skp
  y    15.74%  tabl_vnexpress.skp
  y    15.89%  desk_booking.skp
  y    16.08%  tabl_theverge.skp
  y    16.19%  tabl_googleblog.skp
  y    16.42%  tabl_hsfi.skp
  y    16.49%  tabl_worldjournal.skp
  y    17.03%  tabl_ukwsj.skp
  y    17.33%  tabl_transformice.skp
  y    18.44%  tabl_cnn.skp
  y    19.18%  desk_br337.skp
  y    19.39%  desk_ebay.skp
  y    19.74%  desk_css3gradients.skp
  y    19.75%  desk_googlespreadsheetdashed.skp
  y    19.78%  desk_twitter.skp
  y    20.83%  desk_blogger.skp
  y    20.96%  tabl_sahadan.skp
  y    22.17%  tabl_gamedeksiam.skp
  y    22.33%  tabl_techmeme.skp
  y    23.27%  desk_techcrunch.skp
  y    23.78%  desk_facebook.skp
  y    24.03%  tabl_pravda.skp
  y    25.86%  desk_baidu.skp
  y    26.91%  tabl_nytimes.skp
  y    28.19%  desk_wowwiki.skp
  y    28.22%  tabl_mozilla.skp
  y    28.47%  desk_yahoogames.skp
  y    30.55%  mobi_wikipedia.skp
  y    32.48%  desk_chalkboard.skp
  y    33.45%  desk_pokemonwiki.skp
  y    33.61%  desk_googlespreadsheet.skp
  y    45.51%  desk_linkedin.skp
Overall speedup: 21.91%

~/skia/trunk > compare.sh bench --match picture_record --repeat 100
master vs. dictionaries
sig?  speedup  bench
  y     6.67%  picture_record_unique_paint_dictionary
  y     7.83%  picture_record_dictionaries
  y    39.20%  picture_record_recurring_paint_dictionary
Overall speedup: 17.61%

[tomhudson, 2013-07-24 11:10:14]

lgtm

https://codereview.chromium.org/19564007/diff/28001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/19564007/diff/28001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:413: SkFlatDictionary(SkFlatController* controller,
size_t scratchSizeGuess=0)
Nit: don't we usually put spaces around the =?

https://codereview.chromium.org/19564007/diff/28001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:625: // Reset fScratch to our new larger SkFlatData.
Nit: your CL description is a lot clearer than this comment; I think there could
be a 2-line distillation about *why* instead of just *what*?

[mtklein, 2013-07-24 17:40:15]

https://codereview.chromium.org/19564007/diff/28001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/19564007/diff/28001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:413: SkFlatDictionary(SkFlatController* controller,
size_t scratchSizeGuess=0)
On 2013/07/24 11:10:14, tomhudson wrote:
> Nit: don't we usually put spaces around the =?

Done, and noticed I needed to InitialCaps a static method.

https://codereview.chromium.org/19564007/diff/28001/src/core/SkPictureFlat.h#...
src/core/SkPictureFlat.h:625: // Reset fScratch to our new larger SkFlatData.
On 2013/07/24 11:10:14, tomhudson wrote:
> Nit: your CL description is a lot clearer than this comment; I think there
could
> be a 2-line distillation about *why* instead of just *what*?

How's that?

[commit-bot: I haz the power, 2013-07-24 18:05:43]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/19564007/34001

[commit-bot: I haz the power, 2013-07-24 18:45:29]

Change committed as 10328

[commit-bot: I haz the power, 2013-07-24 20:30:35]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/19564007/39001

[commit-bot: I haz the power, 2013-07-24 20:37:32]

Change committed as 10336

[scroggo, 2013-07-30 21:21:11]

lgtm

https://chromiumcodereview.appspot.com/19564007/diff/39001/src/core/SkPicture...
File src/core/SkPictureFlat.h (right):

https://chromiumcodereview.appspot.com/19564007/diff/39001/src/core/SkPicture...
src/core/SkPictureFlat.h:757: void*                      fLastAllocated;
This should never be a problem in practice, but shouldn't we set fLastAllocated
to NULL in the constructor?