Rearrange SkRecord with small N in mind

Rearrange SkRecord with small N in mind

This rearranges the record pointers and types so they can go in a single array, then preallocates some space for them and for the SkVarAlloc.

picture_overhead_draw bench drops from ~1000ns to 500-600ns, with no effect on picture_overhead_nodraw.

I don't see any significant effect on large picture recording times from our .skps.

BUG=chromium:470553
Committed: https://skia.googlesource.com/skia/+/e2dd9408cd711777afaa9410427fb0d761ab004a

Committed: https://skia.googlesource.com/skia/+/29b1afc169576cf5e708e46b74313b5666e66249

[mtklein_C, 2015-04-08 20:28:50]

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

[mtklein_C, 2015-04-08 20:31:46]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-08 20:32:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1061783002/100001

[commit-bot: I haz the power, 2015-04-08 20:33:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 20:33:46]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[reed1, 2015-04-08 20:34:09]

https://codereview.chromium.org/1061783002/diff/80001/src/core/SkRecord.h
File src/core/SkRecord.h (right):

https://codereview.chromium.org/1061783002/diff/80001/src/core/SkRecord.h#new...
src/core/SkRecord.h:32: kLgInlineAllocBytes = 8, // 256 bytes inline, then
SkVarAlloc mallocs starting at 512 bytes.
the const says "8 bytes" but the comment says 256 bytes. I'm confused what space
kLgInlineAllocBytes is in.

https://codereview.chromium.org/1061783002/diff/80001/src/core/SkRecord.h#new...
src/core/SkRecord.h:38: , fAlloc(kLgInlineAllocBytes+1, fInlineAlloc,
sizeof(fInlineAlloc)) {}
Why the +1 ?

[mtklein, 2015-04-08 20:36:38]

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

[mtklein, 2015-04-08 20:36:39]

https://codereview.chromium.org/1061783002/diff/80001/src/core/SkRecord.h
File src/core/SkRecord.h (right):

https://codereview.chromium.org/1061783002/diff/80001/src/core/SkRecord.h#new...
src/core/SkRecord.h:32: kLgInlineAllocBytes = 8, // 256 bytes inline, then
SkVarAlloc mallocs starting at 512 bytes.
On 2015/04/08 20:34:08, reed1 wrote:
> the const says "8 bytes" but the comment says 256 bytes. I'm confused what
space
> kLgInlineAllocBytes is in.

It's lg(inlineAllocBytes).

https://codereview.chromium.org/1061783002/diff/80001/src/core/SkRecord.h#new...
src/core/SkRecord.h:38: , fAlloc(kLgInlineAllocBytes+1, fInlineAlloc,
sizeof(fInlineAlloc)) {}
On 2015/04/08 20:34:08, reed1 wrote:
> Why the +1 ?

*2.  We inline 1<<8 bytes, then the SkVarAlloc starts at 1<<9.

[reed1, 2015-04-08 20:39:10]

I think both of your answers are good, but should somehow be in the
code/comments.

kInlineBytesPOW2 or kLog2InlineBytes or more detail in comment...

[reed1, 2015-04-08 20:39:49]

lgtm w/ the hope of clearer consts/names/comments for us naive readers.

[mtklein_C, 2015-04-08 20:55:21]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[mtklein_C, 2015-04-08 20:55:22]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1061783002/#ps120001
(title: "manual packing")

[commit-bot: I haz the power, 2015-04-08 20:55:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1061783002/120001

[mtklein_C, 2015-04-08 20:55:59]

On 2015/04/08 20:39:49, reed1 wrote:
> lgtm w/ the hope of clearer consts/names/comments for us naive readers.


Done.  Moved the Lg next to Bytes, and added some more comments.

[commit-bot: I haz the power, 2015-04-08 21:01:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 21:01:46]

This issue passed the CQ dry run.

[mtklein, 2015-04-08 21:01:58]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-04-08 21:02:15]

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

[commit-bot: I haz the power, 2015-04-08 21:02:39]

Committed patchset #8 (id:120001) as
https://skia.googlesource.com/skia/+/e2dd9408cd711777afaa9410427fb0d761ab004a

[mtklein, 2015-04-08 21:09:10]

A revert of this CL (patchset #8 id:120001) has been created in
https://codereview.chromium.org/1068383003/ by mtklein@google.com.

The reason for reverting is:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-G....

[mtklein, 2015-04-09 14:33:38]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-04-09 14:33:38]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1061783002/#ps140001
(title: "32-bit fix")

[commit-bot: I haz the power, 2015-04-09 14:33:49]

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

[mtklein, 2015-04-09 14:45:38]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2015-04-09 14:45:44]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-04-09 14:46:01]

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

[commit-bot: I haz the power, 2015-04-09 14:46:47]

Committed patchset #9 (id:140001) as
https://skia.googlesource.com/skia/+/29b1afc169576cf5e708e46b74313b5666e66249