Reduce sizeof(SkPicture::Analysis) from 24 bytes to 2 bytes.

Reduce sizeof(SkPicture::Analysis) from 24 bytes to 2 bytes.

This saves "up to" 22 bytes per SkPicture.

Due to alignment, this reduces sizeof(SkPicture):
- from 88 to 72 bytes on Linux x64
- from 68 to 48 bytes on Android
(with Chrome's build settings)

It also somewhat simplifies the GPU veto logic.

BUG=skia:

[mtklein] No public API changes.
TBR=reed@google.com

Committed: https://skia.googlesource.com/skia/+/de8667b81f2fed2fbecba68e751756a48df5e0dd

[jbroman, 2015-04-21 20:43:04]

Patchset #1 (id:1) has been deleted

[jbroman, 2015-04-23 17:04:14]

jbroman@chromium.org changed reviewers:
+ mtklein@google.com

[jbroman, 2015-04-23 17:04:14]

Unless there's a reason the Ganesh veto still wants to track all this
separately, this seems like a reasonable simplification that shaves a little off
the size of an SkPicture.

In practice, this saves about 2-3% of SkPicture memory in Chrome w/ Slimming
Paint, which is a modest but not great improvement.

WDYT?

[mtklein, 2015-04-27 13:43:00]

mtklein@google.com changed reviewers:
+ robertphillips@google.com

[mtklein, 2015-04-27 13:43:00]

I think this is a very good idea.  We seem to be profligate with our bytes
today.  Looks like we could even get this down to 3 bits on one uint8_t, just
storing fGPUVeto?

That said, I don't know much about the GPU veto logic itself, or how much it
churns.  Adding Robert.

https://codereview.chromium.org/1060863004/diff/40001/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/1060863004/diff/40001/src/core/SkPicture.cpp#...
src/core/SkPicture.cpp:199: fNumSlowPathsAndDashEffects =
SkTMax<int>(counter.numSlowPathsAndDashEffects, 255);
SkTMin?

[jbroman, 2015-04-27 14:30:50]

On 2015/04/27 13:43:00, mtklein wrote:
> I think this is a very good idea.  We seem to be profligate with our bytes
> today.  Looks like we could even get this down to 3 bits on one uint8_t, just
> storing fGPUVeto?

I don't think so, because want to know "are there at least six slow paths in the
entire SkPicture", so we need to at least store numbers from 0 to 6 unless we
want to recurse -- which I wouldn't want to do, especially since alignment means
we won't gain anything.

SkPicture::Analysis could be made to fit in one byte by putting
fNumSlowPathsAndDashEffects in the bitfield, too (say, of 3 or 4 bits), but we
currently wouldn't get any size back due to alignment, so I'd be inclined to
defer that packing for now.

https://codereview.chromium.org/1060863004/diff/40001/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/1060863004/diff/40001/src/core/SkPicture.cpp#...
src/core/SkPicture.cpp:199: fNumSlowPathsAndDashEffects =
SkTMax<int>(counter.numSlowPathsAndDashEffects, 255);
On 2015/04/27 13:43:00, mtklein wrote:
> SkTMin?

Uh, of course. *facepalm*

I think I had this correct and changed it.

[robertphillips, 2015-04-27 14:41:30]

lgtm + nits

https://codereview.chromium.org/1060863004/diff/60001/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/1060863004/diff/60001/src/core/SkPicture.cpp#...
src/core/SkPicture.cpp:135: 
rm space in "...Effects (0)" ?

https://codereview.chromium.org/1060863004/diff/60001/src/core/SkPicture.cpp#...
src/core/SkPicture.cpp:188: 
Make this fNumSlowPathsAndDashEffects ?

[mtklein, 2015-04-27 14:44:18]

Ah, gotcha.  SGTM.

[jbroman, 2015-04-27 14:48:10]

jbroman@chromium.org changed reviewers:
+ reed@google.com

[jbroman, 2015-04-27 14:48:10]

+reed for skia OWNERS

https://codereview.chromium.org/1060863004/diff/60001/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/1060863004/diff/60001/src/core/SkPicture.cpp#...
src/core/SkPicture.cpp:135: 
On 2015/04/27 14:41:30, robertphillips wrote:
> rm space in "...Effects (0)" ?

Done.

https://codereview.chromium.org/1060863004/diff/60001/src/core/SkPicture.cpp#...
src/core/SkPicture.cpp:188: 
On 2015/04/27 14:41:30, robertphillips wrote:
> Make this fNumSlowPathsAndDashEffects ?

Done. Here and above, I'd been following what the existing code did, but I'm
happy to change.

[robertphillips, 2015-04-27 14:49:36]

robertphillips@google.com changed reviewers:
+ bsalomon@google.com

[robertphillips, 2015-04-27 14:49:37]

Looks like Mike is out today - adding Brian.

[mtklein, 2015-04-27 14:49:49]

mtklein@google.com changed reviewers:
- bsalomon@google.com

[mtklein, 2015-04-27 14:49:49]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-04-27 14:49:49]

lgtm

Just gonna jam this in... there aren't any public API changes here that need
signoff.

[mtklein, 2015-04-27 14:49:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1060863004/#ps80001
(title: "robertphillips nits")

[commit-bot: I haz the power, 2015-04-27 14:49:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1060863004/80001

[commit-bot: I haz the power, 2015-04-27 14:55:36]

Committed patchset #4 (id:80001) as
https://skia.googlesource.com/skia/+/de8667b81f2fed2fbecba68e751756a48df5e0dd