Disable the save/clip/restore peephole optimization when a bounding hierarchy is used

Disable the save/clip/restore peephole optimization when a bounding hierarchy is used

This optimization is redundant with the state traversal optimization that is
performed by SkPictureStateTree and it introduces significant record time
overhead.  Therefore it is better not to perform this optimization when using
a BBoxHierarchy.

BUG=skia:1852
Committed: http://code.google.com/p/skia/source/detail?r=12367

[Justin Novosad, 2013-11-20 21:05:45]

PTAL.

Launched some perf try jobs on Android.  I expect the change to result in better
picture record times when using a tile grid, and I expect all other stats to
remain unchanged.

[reed1, 2013-11-20 21:08:52]

https://codereview.chromium.org/79313003/diff/1/src/core/SkPictureRecord.cpp
File src/core/SkPictureRecord.cpp (right):

https://codereview.chromium.org/79313003/diff/1/src/core/SkPictureRecord.cpp#...
src/core/SkPictureRecord.cpp:526: kSkipIfBBoxHierarchy_Flag = 0x1,  //
Optimization should be skipped if the
Lets augment the comment to explain why (which you did in the CL description,
but would be helpful here too.)

[Justin Novosad, 2013-11-20 21:28:13]

On 2013/11/20 21:08:52, reed1 wrote:
> https://codereview.chromium.org/79313003/diff/1/src/core/SkPictureRecord.cpp
> File src/core/SkPictureRecord.cpp (right):
> 
>
https://codereview.chromium.org/79313003/diff/1/src/core/SkPictureRecord.cpp#...
> src/core/SkPictureRecord.cpp:526: kSkipIfBBoxHierarchy_Flag = 0x1,  //
> Optimization should be skipped if the
> Lets augment the comment to explain why (which you did in the CL description,
> but would be helpful here too.)

Done.  I put the additional comments where the flag is used (as opposed to where
it is declared).
Since this is a comment-only change, I did not send new perf try jobs.  Let's
just keep an eye on the jobs tied to Patch Set 1

[tomhudson, 2013-11-21 11:46:16]

Code LGTM, I can't see the trybots.

Nit: is it clear that fBoundingHierarchy != NULL <-> we have a BBoxHierarchy?
Will that always be true?

[robertphillips, 2013-11-21 13:02:24]

lgtm

[commit-bot: I haz the power, 2013-11-21 13:03:49]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/junov@chromium.org/79313003/90001

[commit-bot: I haz the power, 2013-11-21 13:21:44]

Retried try job too often on Build-Mac10.7-Clang-x86-Release-Trybot for step(s)
BuildEverything, BuildSkiaLib, BuildTests
http://skiabot-master.pogerlabs.com:10117/buildstatus?builder=Build-Mac10.7-C...

[Justin Novosad, 2013-11-22 19:22:38]

On 2013/11/21 13:21:44, I haz the power (commit-bot) wrote:
> Retried try job too often on Build-Mac10.7-Clang-x86-Release-Trybot for
step(s)
> BuildEverything, BuildSkiaLib, BuildTests
>
http://skiabot-master.pogerlabs.com:10117/buildstatus?builder=Build-Mac10.7-C...

Compared try jobs to results on the bots.  There seems to be no impact on
playback perf.  The tile grid record times are lower or equal.  
About 5% record time perf win on tabl_cuteoverload with Nexus7.

Will fix-up patch and commit.

[commit-bot: I haz the power, 2013-11-22 19:31:48]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/junov@chromium.org/79313003/270001

[commit-bot: I haz the power, 2013-11-22 19:42:15]

Change committed as 12367