Prune SkRTree

Prune SkRTree

  - Propagate a bunch of constant parameters through.
  - Delete code that's not used when bulk loading.
  - Allocate all Nodes together.
  - Stay in SkRect.

Doing a single malloc for the nodes can't not have improved memory usage.

Looks like this might improve record performance ~5%, probably mostly from
staying in SkRects.  This finally dethrones building the BBH as the hot spot.
(Now it's mapping user bounds back to device bounds and adjusting for paints.)

Recording time changes from my MBP:
    desk_rectangletransition.skp	11.5us -> 11.7us	1x
             desk_forecastio.skp	 115us ->  114us	0.98x
                desk_booking.skp	 550us ->  541us	0.98x
            tabl_mercurynews.skp	 176us ->  173us	0.98x
                   tabl_hsfi.skp	 294us ->  287us	0.98x
              desk_wordpress.skp	 351us ->  343us	0.98x
           tabl_worldjournal.skp	 439us ->  426us	0.97x
                  tabl_gmail.skp	20.3us -> 19.7us	0.97x
         desk_youtubetvvideo.skp	10.8us -> 10.4us	0.97x
             desk_googleplus.skp	 1.1ms -> 1.07ms	0.97x
               tabl_slashdot.skp	 106us ->  103us	0.97x
         desk_jsfiddlebigcar.skp	26.7us -> 25.7us	0.96x
               tabl_techmeme.skp	95.4us -> 91.7us	0.96x
             tabl_deviantart.skp	 133us ->  127us	0.96x
              desk_pinterest.skp	40.6us -> 38.9us	0.96x
                 desk_carsvg.skp	 195us ->  187us	0.96x
               tabl_engadget.skp	 376us ->  359us	0.96x
                tabl_sahadan.skp	60.5us -> 57.5us	0.95x
      tabl_culturalsolutions.skp	 255us ->  242us	0.95x
                  tabl_gspro.skp	58.3us -> 55.5us	0.95x
               desk_linkedin.skp	 146us ->  138us	0.94x
                   desk_ebay.skp	 192us ->  181us	0.94x
                    tabl_cnn.skp	 467us ->  440us	0.94x
     desk_jsfiddlehumperclip.skp	29.9us -> 28.1us	0.94x
               desk_tigersvg.skp	43.2us -> 40.5us	0.94x
           desk_yahooanswers.skp	 131us ->  123us	0.94x
desk_googlespreadsheetdashed.skp	1.18ms -> 1.11ms	0.94x
                desk_blogger.skp	 193us ->  181us	0.94x
                tabl_mozilla.skp	1.82ms ->  1.7ms	0.94x
                    tabl_mlb.skp	 145us ->  136us	0.93x
              mobi_wikipedia.skp	 577us ->  539us	0.93x
               tabl_frantzen.skp	54.1us -> 50.4us	0.93x
                  desk_baidu.skp	87.9us -> 81.9us	0.93x
             desk_techcrunch.skp	 224us ->  209us	0.93x
                 desk_sfgate.skp	 206us ->  192us	0.93x
                  tabl_ukwsj.skp	 269us ->  250us	0.93x
               desk_facebook.skp	 316us ->  293us	0.93x
            desk_gmailthread.skp	 205us ->  190us	0.93x
         tabl_googlecalendar.skp	 158us ->  147us	0.93x
                   tabl_digg.skp	 382us ->  354us	0.93x
                 desk_amazon.skp	 106us -> 98.5us	0.93x
          tabl_androidpolice.skp	 693us ->  642us	0.93x
                tabl_nytimes.skp	 206us ->  191us	0.92x
                    desk_gws.skp	 124us ->  114us	0.92x
                desk_youtube.skp	 255us ->  235us	0.92x
           tabl_cuteoverload.skp	 583us ->  537us	0.92x
            desk_oldinboxapp.skp	  18us -> 16.6us	0.92x
             desk_mobilenews.skp	 297us ->  273us	0.92x
                 tabl_pravda.skp	 168us ->  154us	0.92x
              tabl_vnexpress.skp	 236us ->  217us	0.92x
          desk_css3gradients.skp	 202us ->  185us	0.92x
            tabl_gamedeksiam.skp	 508us ->  464us	0.91x
                desk_wowwiki.skp	1.02ms ->  929us	0.91x
                   desk_espn.skp	 209us ->  191us	0.91x
             desk_chalkboard.skp	 315us ->  284us	0.9x
                 desk_mapsvg.skp	 607us ->  543us	0.89x
            desk_pokemonwiki.skp	5.18ms -> 4.62ms	0.89x
               desk_samoasvg.skp	 335us ->  298us	0.89x
        desk_youtubetvbrowse.skp	10.1us -> 8.59us	0.85x
BUG=skia:3085, skia:2834
Committed: https://skia.googlesource.com/skia/+/a06a9531213d2f00a0fe1dc07acd96eba57e6044

[mtklein, 2014-11-18 00:29:16]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-11-18 00:39:47]

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

[commit-bot: I haz the power, 2014-11-18 00:39:48]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-11-18 06:39 UTC

[commit-bot: I haz the power, 2014-11-18 00:47:59]

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

[commit-bot: I haz the power, 2014-11-18 00:48:00]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-Shared-Trybot on client.skia
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...)

[mtklein, 2014-11-18 00:49:59]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-11-18 00:50:48]

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

[commit-bot: I haz the power, 2014-11-18 00:50:49]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-11-18 06:50 UTC

[mtklein, 2014-11-18 01:07:51]

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

[commit-bot: I haz the power, 2014-11-18 06:50:55]

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

[commit-bot: I haz the power, 2014-11-18 06:50:56]

No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer from
https://skia.googlesource.com/skia/+/master/CQ_COMMITTERS

[robertphillips, 2014-11-18 16:02:15]

Here is my first round of comments. I want to take another look.

https://codereview.chromium.org/734723002/diff/120001/bench/RTreeBench.cpp
File bench/RTreeBench.cpp (right):

https://codereview.chromium.org/734723002/diff/120001/bench/RTreeBench.cpp#ne...
bench/RTreeBench.cpp:59: RTreeQueryBench(const char* name, MakeRectProc proc)
put this on prior line ?

https://codereview.chromium.org/734723002/diff/120001/src/core/SkBBHFactory.cpp
File src/core/SkBBHFactory.cpp (right):

https://codereview.chromium.org/734723002/diff/120001/src/core/SkBBHFactory.c...
src/core/SkBBHFactory.cpp:13: SkBBoxHierarchy* SkRTreeFactory::operator()(int
width, int height) const {
single line ?

https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp
File src/core/SkRTree.cpp (right):

https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp#ne...
src/core/SkRTree.cpp:18: for (int i = 0; i < N; i++) {
const SkRect& bounds ?

https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp#ne...
src/core/SkRTree.cpp:30: if (fCount) {
keep the yoda speak ?

https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp#ne...
src/core/SkRTree.cpp:161: 
const SkRect& ?

https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.h
File src/core/SkRTree.h (right):

https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.h#newc...
src/core/SkRTree.h:73: 
const SkRect& ?

[mtklein, 2014-11-18 16:05:50]

On 2014/11/18 16:02:15, robertphillips wrote:
> Here is my first round of comments. I want to take another look.
> 
> https://codereview.chromium.org/734723002/diff/120001/bench/RTreeBench.cpp
> File bench/RTreeBench.cpp (right):
> 
>
https://codereview.chromium.org/734723002/diff/120001/bench/RTreeBench.cpp#ne...
> bench/RTreeBench.cpp:59: RTreeQueryBench(const char* name, MakeRectProc proc)
> put this on prior line ?
> 
>
https://codereview.chromium.org/734723002/diff/120001/src/core/SkBBHFactory.cpp
> File src/core/SkBBHFactory.cpp (right):
> 
>
https://codereview.chromium.org/734723002/diff/120001/src/core/SkBBHFactory.c...
> src/core/SkBBHFactory.cpp:13: SkBBoxHierarchy* SkRTreeFactory::operator()(int
> width, int height) const {
> single line ?
> 
> https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp
> File src/core/SkRTree.cpp (right):
> 
>
https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp#ne...
> src/core/SkRTree.cpp:18: for (int i = 0; i < N; i++) {
> const SkRect& bounds ?
> 
>
https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp#ne...
> src/core/SkRTree.cpp:30: if (fCount) {
> keep the yoda speak ?
> 
>
https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.cpp#ne...
> src/core/SkRTree.cpp:161: 
> const SkRect& ?
> 
> https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.h
> File src/core/SkRTree.h (right):
> 
>
https://codereview.chromium.org/734723002/diff/120001/src/core/SkRTree.h#newc...
> src/core/SkRTree.h:73: 
> const SkRect& ?

Done.

[mtklein, 2014-11-18 16:05:57]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-11-18 16:15:08]

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

[commit-bot: I haz the power, 2014-11-18 16:15:09]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-11-18 22:15 UTC

[robertphillips, 2014-11-18 17:26:43]

lgtm + a question

https://codereview.chromium.org/734723002/diff/140001/src/core/SkRTree.h
File src/core/SkRTree.h (right):

https://codereview.chromium.org/734723002/diff/140001/src/core/SkRTree.h#newc...
src/core/SkRTree.h:69: uint16_t fNumChildren;
Can we do with out fLevel now ?

[commit-bot: I haz the power, 2014-11-18 17:27:54]

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

[mtklein, 2014-11-18 17:34:00]

https://codereview.chromium.org/734723002/diff/140001/src/core/SkRTree.h
File src/core/SkRTree.h (right):

https://codereview.chromium.org/734723002/diff/140001/src/core/SkRTree.h#newc...
src/core/SkRTree.h:69: uint16_t fNumChildren;
On 2014/11/18 17:26:43, robertphillips wrote:
> Can we do with out fLevel now ?

Yes, I think I can get rid of that.  Didn't bother yet because it doesn't save
us anything without also getting rid of or moving fNumChildren.