Merge tomhudson and mtklein SkPaint shrinking approaches.

Merge tomhudson and mtklein SkPaint shrinking approaches.

I think this is cherry picking the best parts of both our CLs.  We've got dirty bit tracking from Tom's, picture format stability from Mike's, etc.  Paints are typically 1/3 their original size when flattened in the dictionary.

bench_record on my desktop looks promising.  Generally, looks faster.  (Best in monospace.)
a/b   skp                               before  after
0.83  desk_techcrunch.skp               0.29    0.24
0.83  tabl_gamedeksiam.skp              0.52    0.43
0.87  desk_carsvg.skp                   0.4     0.35
0.87  desk_googlehome.skp               0.038   0.033
0.87  desk_pokemonwiki.skp              3.9     3.4
0.88  desk_fontwipe.skp                 0.0089  0.0078
0.88  desk_googlespreadsheet.skp        0.16    0.14
0.89  desk_jsfiddlebigcar.skp           0.027   0.024
0.89  desk_tigersvg.skp                 0.038   0.034
0.89  desk_weather.skp                  0.19    0.17
0.89  tabl_engadget.skp                 0.37    0.33
0.89  tabl_googleblog.skp               0.28    0.25
0.9   desk_facebook.skp                 0.2     0.18
0.91  desk_mapsvg.skp                   0.45    0.41
0.91  desk_youtube.skp                  0.22    0.2
0.92  desk_forecastio.skp               0.12    0.11
0.92  desk_googlespreadsheetdashed.skp  0.49    0.45
0.92  desk_gws.skp                      0.13    0.12
0.92  desk_pinterest.skp                0.037   0.034
0.92  desk_twitter.skp                  0.25    0.23
0.92  tabl_culturalsolutions.skp        0.26    0.24
0.92  tabl_gspro.skp                    0.072   0.066
0.92  tabl_mercurynews.skp              0.26    0.24
0.93  desk_booking.skp                  0.46    0.43
0.93  desk_chalkboard.skp               0.28    0.26
0.93  desk_linkedin.skp                 0.14    0.13
0.93  desk_mobilenews.skp               0.28    0.26
0.93  tabl_cuteoverload.skp             0.46    0.43
0.93  tabl_deviantart.skp               0.15    0.14
0.93  tabl_gmail.skp                    0.029   0.027
0.93  tabl_googlecalendar.skp           0.15    0.14
0.93  tabl_mlb.skp                      0.15    0.14
0.94  desk_blogger.skp                  0.18    0.17
0.94  desk_jsfiddlehumperclip.skp       0.034   0.032
0.94  desk_wordpress.skp                0.33    0.31
0.94  desk_wowwiki.skp                  0.94    0.88
0.94  desk_yahooanswers.skp             0.17    0.16
0.94  desk_youtubetvvideo.skp           0.017   0.016
0.94  tabl_sahadan.skp                  0.093   0.087
0.94  tabl_worldjournal.skp             0.35    0.33
0.95  desk_css3gradients.skp            0.21    0.2
0.95  desk_gmailthread.skp              0.19    0.18
0.95  tabl_cnet.skp                     0.42    0.4
0.95  tabl_mozilla.skp                  1.9     1.8
0.95  tabl_pravda.skp                   0.19    0.18
0.96  mobi_wikipedia.skp                0.55    0.53
0.96  tabl_cnn.skp                      0.48    0.46
0.96  tabl_nofolo.skp                   0.05    0.048
0.97  desk_googleplus.skp               0.29    0.28
0.97  tabl_frantzen.skp                 0.059   0.057
0.97  tabl_onlinewsj.skp                0.38    0.37
0.97  tabl_slashdot.skp                 0.1     0.097
0.97  tabl_vnexpress.skp                0.29    0.28
0.99  desk_amazon.skp                   0.088   0.087
1     desk_baidu.skp                    0.097   0.099
1     desk_ebay.skp                     0.18    0.18
1     desk_espn.skp                     0.24    0.24
1     desk_oldinboxapp.skp              0.026   0.026
1     desk_rectangletransition.skp      0.014   0.014
1     desk_samoasvg.skp                 0.23    0.24
1     desk_yahoogames.skp               0.029   0.029
1     desk_yahoosports.skp              0.0033  0.0033
1     desk_youtubetvbrowse.skp          0.01    0.01
1     tabl_androidpolice.skp            0.65    0.65
1     tabl_digg.skp                     0.33    0.33
1     tabl_hsfi.skp                     0.32    0.32
1     tabl_nytimes.skp                  0.22    0.22
1     tabl_techmeme.skp                 0.069   0.072
1     tabl_ukwsj.skp                    0.35    0.35
1.1   desk_sfgate.skp                   0.25    0.28


BUG=skia:2190, skia:2194
Committed: http://code.google.com/p/skia/source/detail?r=13487

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

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

[mtklein_C, 2014-02-10 21:21:04]

Have a look?  Sorry to keep jumping around on CLs like this... it's become a
real pain (impossible?) to work on a single CL from different accounts.

[tomhudson, 2014-02-11 13:49:08]

Dag nabbit, I need to fix my filters - didn't see this all day.

[tomhudson, 2014-02-11 14:30:07]

Profile looks a little better:

+  10.25%  bench_record  libc-2.15.so                [.] __memcpy_ssse3_back
+   5.84%  bench_record  bench_record                [.]
SkPicturePlayback::draw(SkCanvas&, SkDrawPic
+   4.94%  bench_record  bench_record                [.]
SkPictureRecord::drawRect(SkRect const&, SkP
+   4.29%  bench_record  bench_record                [.]
SkPaint::FlatteningTraits::flatten(SkWriteBu
+   4.05%  bench_record  bench_record                [.]
SkPictureRecord::drawPosTextHImpl(void const
+   4.00%  bench_record  bench_record                [.]
SkPictureRecord::drawPosTextH(void const*, u
+   3.65%  bench_record  libgcc_s.so.1               [.] __popcountdi2
+   2.76%  bench_record  libc-2.15.so                [.] _int_malloc
+   2.23%  bench_record  libGL.so.319.32             [.] 0xabecd
+   2.20%  bench_record  bench_record                [.] SkPtrSet::add(void*)
+   1.83%  bench_record  bench_record                [.]
chromium_jpeg_idct_islow
+   1.83%  bench_record  libc-2.15.so                [.] __memcpy_sse2
+   1.72%  bench_record  libc-2.15.so                [.] __memcmp_sse4_1
+   1.65%  bench_record  libnvidia-tls.so.319.32     [.] 0x1f6c
+   1.44%  bench_record  bench_record                [.]
SkWriter32::growToAtLeast(unsigned long)
+   1.42%  bench_record  libz.so.1.2.3.4             [.] inflate_fast
+   1.23%  bench_record  libpng12.so.0.46.0          [.] 0xbeea
+   1.17%  bench_record  bench_record                [.]
SkPictureRecord::drawPath(SkPath const&, SkP
+   1.14%  bench_record  bench_record                [.]
SkGlyphCache::VisitCache(SkTypeface*, SkDesc
+   1.11%  bench_record  bench_record                [.]
SkPictureRecord::drawBitmapRectToRect(SkBitm
+   1.01%  bench_record  bench_record                [.]
SkWriteBuffer::writeTypeface(SkTypeface*)
+   1.01%  bench_record  bench_record                [.]
SkClipStack::Element::updateBoundAndGenID(Sk

If I turn off inlining with compiler arguments I see a lot of time in:

SkWriter32::reserve() - so hopefully Ian's patches will give us more wins there
SkFlatData::operator==()
SkTDynamicHash<SkFlatData>::find()
SkChecksum::Compute()
SkChecksum::Mash()
SkWriter32::write32()
SkReader32::readInt()
SkFlatDictionary<SkPaint>::resetScratch()
SkPictureRecord::addDraw()
SkTDynamicHash<SkFlatData>::nextIndex()

(Ordering not stable across runs, so no particular order.) At least some of this
is performance-loss-due-to-lack-of-inlining, rather than performance issues that
were hidden by inlining. I guess Ian's focused hackery of manually deinlining
suspect functions might be necessary to get clear signal.

[mtklein_C, 2014-02-11 17:45:12]

On 2014/02/11 14:30:07, tomhudson wrote:
> Profile looks a little better:
> 
> +  10.25%  bench_record  libc-2.15.so                [.] __memcpy_ssse3_back
> +   5.84%  bench_record  bench_record                [.]
> SkPicturePlayback::draw(SkCanvas&, SkDrawPic
> +   4.94%  bench_record  bench_record                [.]
> SkPictureRecord::drawRect(SkRect const&, SkP
> +   4.29%  bench_record  bench_record                [.]
> SkPaint::FlatteningTraits::flatten(SkWriteBu
> +   4.05%  bench_record  bench_record                [.]
> SkPictureRecord::drawPosTextHImpl(void const
> +   4.00%  bench_record  bench_record                [.]
> SkPictureRecord::drawPosTextH(void const*, u
> +   3.65%  bench_record  libgcc_s.so.1               [.] __popcountdi2
> +   2.76%  bench_record  libc-2.15.so                [.] _int_malloc
> +   2.23%  bench_record  libGL.so.319.32             [.] 0xabecd
> +   2.20%  bench_record  bench_record                [.] SkPtrSet::add(void*)
> +   1.83%  bench_record  bench_record                [.]
> chromium_jpeg_idct_islow
> +   1.83%  bench_record  libc-2.15.so                [.] __memcpy_sse2
> +   1.72%  bench_record  libc-2.15.so                [.] __memcmp_sse4_1
> +   1.65%  bench_record  libnvidia-tls.so.319.32     [.] 0x1f6c
> +   1.44%  bench_record  bench_record                [.]
> SkWriter32::growToAtLeast(unsigned long)
> +   1.42%  bench_record  libz.so.1.2.3.4             [.] inflate_fast
> +   1.23%  bench_record  libpng12.so.0.46.0          [.] 0xbeea
> +   1.17%  bench_record  bench_record                [.]
> SkPictureRecord::drawPath(SkPath const&, SkP
> +   1.14%  bench_record  bench_record                [.]
> SkGlyphCache::VisitCache(SkTypeface*, SkDesc
> +   1.11%  bench_record  bench_record                [.]
> SkPictureRecord::drawBitmapRectToRect(SkBitm
> +   1.01%  bench_record  bench_record                [.]
> SkWriteBuffer::writeTypeface(SkTypeface*)
> +   1.01%  bench_record  bench_record                [.]
> SkClipStack::Element::updateBoundAndGenID(Sk
> 
> If I turn off inlining with compiler arguments I see a lot of time in:
> 
> SkWriter32::reserve() - so hopefully Ian's patches will give us more wins
there
> SkFlatData::operator==()
> SkTDynamicHash<SkFlatData>::find()
> SkChecksum::Compute()
> SkChecksum::Mash()
> SkWriter32::write32()
> SkReader32::readInt()
> SkFlatDictionary<SkPaint>::resetScratch()
> SkPictureRecord::addDraw()
> SkTDynamicHash<SkFlatData>::nextIndex()
> 
> (Ordering not stable across runs, so no particular order.) At least some of
this
> is performance-loss-due-to-lack-of-inlining, rather than performance issues
that
> were hidden by inlining. I guess Ian's focused hackery of manually deinlining
> suspect functions might be necessary to get clear signal.

OK, spent the morning polishing the CL.  Looks presentable to me now.

I reran bench_record on my iMac.  Same general results: relative runtimes in the
80-110% range, though now with weird outliers at 67% and 170%.
0.67  tabl_cnn.skp                      0.82    0.55
0.79  desk_yahoosports.skp              0.0056  0.0044
0.84  desk_samoasvg.skp                 0.31    0.26
0.88  desk_youtubetvvideo.skp           0.026   0.023
0.9   desk_googlehome.skp               0.049   0.044
0.91  desk_chalkboard.skp               0.34    0.31
0.92  desk_fontwipe.skp                 0.013   0.012
0.92  tabl_gamedeksiam.skp              0.59    0.54
0.92  tabl_slashdot.skp                 0.13    0.12
0.93  desk_forecastio.skp               0.15    0.14
0.93  tabl_techmeme.skp                 0.092   0.086
0.94  desk_youtubetvbrowse.skp          0.018   0.017
0.95  desk_gmailthread.skp              0.22    0.21
0.95  desk_yahoogames.skp               0.039   0.037
0.95  mobi_wikipedia.skp                0.87    0.83
0.96  desk_blogger.skp                  0.23    0.22
0.96  desk_carsvg.skp                   0.45    0.43
0.97  desk_wordpress.skp                0.39    0.38
0.97  tabl_mercurynews.skp              0.3     0.29
0.98  desk_pokemonwiki.skp              5.1     5
0.98  desk_tigersvg.skp                 0.048   0.047
0.98  tabl_cnet.skp                     0.5     0.49
0.98  tabl_gmail.skp                    0.042   0.041
0.99  tabl_nofolo.skp                   0.068   0.067
1     NULL                              0.01    0.01
1     desk_baidu.skp                    0.12    0.12
1     desk_booking.skp                  0.62    0.62
1     desk_css3gradients.skp            0.25    0.25
1     desk_espn.skp                     0.28    0.29
1     desk_facebook.skp                 0.22    0.22
1     desk_googlespreadsheet.skp        0.17    0.17
1     desk_googlespreadsheetdashed.skp  0.54    0.56
1     desk_gws.skp                      0.16    0.16
1     desk_linkedin.skp                 0.17    0.17
1     desk_mobilenews.skp               0.33    0.33
1     desk_oldinboxapp.skp              0.034   0.034
1     desk_pinterest.skp                0.048   0.048
1     desk_rectangletransition.skp      0.02    0.02
1     desk_sfgate.skp                   0.3     0.31
1     desk_techcrunch.skp               0.32    0.32
1     desk_twitter.skp                  0.29    0.29
1     desk_weather.skp                  0.21    0.22
1     desk_wowwiki.skp                  1.3     1.3
1     desk_youtube.skp                  0.24    0.24
1     tabl_androidpolice.skp            0.98    1
1     tabl_culturalsolutions.skp        0.32    0.32
1     tabl_cuteoverload.skp             0.51    0.52
1     tabl_deviantart.skp               0.18    0.18
1     tabl_digg.skp                     0.4     0.4
1     tabl_frantzen.skp                 0.078   0.079
1     tabl_googleblog.skp               0.32    0.32
1     tabl_googlecalendar.skp           0.17    0.17
1     tabl_gspro.skp                    0.09    0.092
1     tabl_hsfi.skp                     0.39    0.39
1     tabl_mlb.skp                      0.18    0.18
1     tabl_mozilla.skp                  2.8     2.9
1     tabl_sahadan.skp                  0.12    0.12
1     tabl_ukwsj.skp                    0.41    0.42
1     tabl_vnexpress.skp                0.34    0.35
1     tabl_worldjournal.skp             0.39    0.4
1.1   desk_amazon.skp                   0.11    0.12
1.1   desk_googleplus.skp               0.33    0.35
1.1   desk_jsfiddlebigcar.skp           0.039   0.041
1.1   desk_jsfiddlehumperclip.skp       0.047   0.053
1.1   desk_mapsvg.skp                   0.51    0.54
1.1   desk_yahooanswers.skp             0.15    0.16
1.1   tabl_engadget.skp                 0.41    0.44
1.1   tabl_nytimes.skp                  0.28    0.31
1.1   tabl_onlinewsj.skp                0.44    0.47
1.1   tabl_pravda.skp                   0.23    0.25
1.7   desk_ebay.skp                     0.23    0.4

I'm seeing qualitatively the same profile as Tom:
  1) memmove (copying text)
  2) draw (tool overhead)
  3) memcmp (hash table final check)
  4) drawPosTextHImpl (caching text metrics for playback)
  5) flatten
  6) SkPtrSet::add (flatten typefaces)
  7) findAndReturnMutableFlat (hash table lookup)
  8) something in libz (tool overhead)
  9) realloc

[mtklein_C, 2014-02-11 17:49:37]

On 2014/02/11 17:45:12, mtklein_C wrote:
> On 2014/02/11 14:30:07, tomhudson wrote:
> > Profile looks a little better:
> > 
> > +  10.25%  bench_record  libc-2.15.so                [.] __memcpy_ssse3_back
> > +   5.84%  bench_record  bench_record                [.]
> > SkPicturePlayback::draw(SkCanvas&, SkDrawPic
> > +   4.94%  bench_record  bench_record                [.]
> > SkPictureRecord::drawRect(SkRect const&, SkP
> > +   4.29%  bench_record  bench_record                [.]
> > SkPaint::FlatteningTraits::flatten(SkWriteBu
> > +   4.05%  bench_record  bench_record                [.]
> > SkPictureRecord::drawPosTextHImpl(void const
> > +   4.00%  bench_record  bench_record                [.]
> > SkPictureRecord::drawPosTextH(void const*, u
> > +   3.65%  bench_record  libgcc_s.so.1               [.] __popcountdi2
> > +   2.76%  bench_record  libc-2.15.so                [.] _int_malloc
> > +   2.23%  bench_record  libGL.so.319.32             [.] 0xabecd
> > +   2.20%  bench_record  bench_record                [.]
SkPtrSet::add(void*)
> > +   1.83%  bench_record  bench_record                [.]
> > chromium_jpeg_idct_islow
> > +   1.83%  bench_record  libc-2.15.so                [.] __memcpy_sse2
> > +   1.72%  bench_record  libc-2.15.so                [.] __memcmp_sse4_1
> > +   1.65%  bench_record  libnvidia-tls.so.319.32     [.] 0x1f6c
> > +   1.44%  bench_record  bench_record                [.]
> > SkWriter32::growToAtLeast(unsigned long)
> > +   1.42%  bench_record  libz.so.1.2.3.4             [.] inflate_fast
> > +   1.23%  bench_record  libpng12.so.0.46.0          [.] 0xbeea
> > +   1.17%  bench_record  bench_record                [.]
> > SkPictureRecord::drawPath(SkPath const&, SkP
> > +   1.14%  bench_record  bench_record                [.]
> > SkGlyphCache::VisitCache(SkTypeface*, SkDesc
> > +   1.11%  bench_record  bench_record                [.]
> > SkPictureRecord::drawBitmapRectToRect(SkBitm
> > +   1.01%  bench_record  bench_record                [.]
> > SkWriteBuffer::writeTypeface(SkTypeface*)
> > +   1.01%  bench_record  bench_record                [.]
> > SkClipStack::Element::updateBoundAndGenID(Sk
> > 
> > If I turn off inlining with compiler arguments I see a lot of time in:
> > 
> > SkWriter32::reserve() - so hopefully Ian's patches will give us more wins
> there
> > SkFlatData::operator==()
> > SkTDynamicHash<SkFlatData>::find()
> > SkChecksum::Compute()
> > SkChecksum::Mash()
> > SkWriter32::write32()
> > SkReader32::readInt()
> > SkFlatDictionary<SkPaint>::resetScratch()
> > SkPictureRecord::addDraw()
> > SkTDynamicHash<SkFlatData>::nextIndex()
> > 
> > (Ordering not stable across runs, so no particular order.) At least some of
> this
> > is performance-loss-due-to-lack-of-inlining, rather than performance issues
> that
> > were hidden by inlining. I guess Ian's focused hackery of manually
deinlining
> > suspect functions might be necessary to get clear signal.
> 
> OK, spent the morning polishing the CL.  Looks presentable to me now.
> 
> I reran bench_record on my iMac.  Same general results: relative runtimes in
the
> 80-110% range, though now with weird outliers at 67% and 170%.
> 0.67  tabl_cnn.skp                      0.82    0.55
> 0.79  desk_yahoosports.skp              0.0056  0.0044
> 0.84  desk_samoasvg.skp                 0.31    0.26
> 0.88  desk_youtubetvvideo.skp           0.026   0.023
> 0.9   desk_googlehome.skp               0.049   0.044
> 0.91  desk_chalkboard.skp               0.34    0.31
> 0.92  desk_fontwipe.skp                 0.013   0.012
> 0.92  tabl_gamedeksiam.skp              0.59    0.54
> 0.92  tabl_slashdot.skp                 0.13    0.12
> 0.93  desk_forecastio.skp               0.15    0.14
> 0.93  tabl_techmeme.skp                 0.092   0.086
> 0.94  desk_youtubetvbrowse.skp          0.018   0.017
> 0.95  desk_gmailthread.skp              0.22    0.21
> 0.95  desk_yahoogames.skp               0.039   0.037
> 0.95  mobi_wikipedia.skp                0.87    0.83
> 0.96  desk_blogger.skp                  0.23    0.22
> 0.96  desk_carsvg.skp                   0.45    0.43
> 0.97  desk_wordpress.skp                0.39    0.38
> 0.97  tabl_mercurynews.skp              0.3     0.29
> 0.98  desk_pokemonwiki.skp              5.1     5
> 0.98  desk_tigersvg.skp                 0.048   0.047
> 0.98  tabl_cnet.skp                     0.5     0.49
> 0.98  tabl_gmail.skp                    0.042   0.041
> 0.99  tabl_nofolo.skp                   0.068   0.067
> 1     NULL                              0.01    0.01
> 1     desk_baidu.skp                    0.12    0.12
> 1     desk_booking.skp                  0.62    0.62
> 1     desk_css3gradients.skp            0.25    0.25
> 1     desk_espn.skp                     0.28    0.29
> 1     desk_facebook.skp                 0.22    0.22
> 1     desk_googlespreadsheet.skp        0.17    0.17
> 1     desk_googlespreadsheetdashed.skp  0.54    0.56
> 1     desk_gws.skp                      0.16    0.16
> 1     desk_linkedin.skp                 0.17    0.17
> 1     desk_mobilenews.skp               0.33    0.33
> 1     desk_oldinboxapp.skp              0.034   0.034
> 1     desk_pinterest.skp                0.048   0.048
> 1     desk_rectangletransition.skp      0.02    0.02
> 1     desk_sfgate.skp                   0.3     0.31
> 1     desk_techcrunch.skp               0.32    0.32
> 1     desk_twitter.skp                  0.29    0.29
> 1     desk_weather.skp                  0.21    0.22
> 1     desk_wowwiki.skp                  1.3     1.3
> 1     desk_youtube.skp                  0.24    0.24
> 1     tabl_androidpolice.skp            0.98    1
> 1     tabl_culturalsolutions.skp        0.32    0.32
> 1     tabl_cuteoverload.skp             0.51    0.52
> 1     tabl_deviantart.skp               0.18    0.18
> 1     tabl_digg.skp                     0.4     0.4
> 1     tabl_frantzen.skp                 0.078   0.079
> 1     tabl_googleblog.skp               0.32    0.32
> 1     tabl_googlecalendar.skp           0.17    0.17
> 1     tabl_gspro.skp                    0.09    0.092
> 1     tabl_hsfi.skp                     0.39    0.39
> 1     tabl_mlb.skp                      0.18    0.18
> 1     tabl_mozilla.skp                  2.8     2.9
> 1     tabl_sahadan.skp                  0.12    0.12
> 1     tabl_ukwsj.skp                    0.41    0.42
> 1     tabl_vnexpress.skp                0.34    0.35
> 1     tabl_worldjournal.skp             0.39    0.4
> 1.1   desk_amazon.skp                   0.11    0.12
> 1.1   desk_googleplus.skp               0.33    0.35
> 1.1   desk_jsfiddlebigcar.skp           0.039   0.041
> 1.1   desk_jsfiddlehumperclip.skp       0.047   0.053
> 1.1   desk_mapsvg.skp                   0.51    0.54
> 1.1   desk_yahooanswers.skp             0.15    0.16
> 1.1   tabl_engadget.skp                 0.41    0.44
> 1.1   tabl_nytimes.skp                  0.28    0.31
> 1.1   tabl_onlinewsj.skp                0.44    0.47
> 1.1   tabl_pravda.skp                   0.23    0.25
> 1.7   desk_ebay.skp                     0.23    0.4
> 
> I'm seeing qualitatively the same profile as Tom:
>   1) memmove (copying text)
>   2) draw (tool overhead)
>   3) memcmp (hash table final check)
>   4) drawPosTextHImpl (caching text metrics for playback)
>   5) flatten
>   6) SkPtrSet::add (flatten typefaces)
>   7) findAndReturnMutableFlat (hash table lookup)
>   8) something in libz (tool overhead)
>   9) realloc

Hmm.  Reran those outliers.  They both look like noise.  I'm seeing desk_ebay as
~0.24 and table_cnn as ~0.76 reliably on both here and on a clean branch.

[mtklein, 2014-02-13 19:56:36]

On 2014/02/11 17:49:37, mtklein_C wrote:
> On 2014/02/11 17:45:12, mtklein_C wrote:
> > On 2014/02/11 14:30:07, tomhudson wrote:
> > > Profile looks a little better:
> > > 
> > > +  10.25%  bench_record  libc-2.15.so                [.]
__memcpy_ssse3_back
> > > +   5.84%  bench_record  bench_record                [.]
> > > SkPicturePlayback::draw(SkCanvas&, SkDrawPic
> > > +   4.94%  bench_record  bench_record                [.]
> > > SkPictureRecord::drawRect(SkRect const&, SkP
> > > +   4.29%  bench_record  bench_record                [.]
> > > SkPaint::FlatteningTraits::flatten(SkWriteBu
> > > +   4.05%  bench_record  bench_record                [.]
> > > SkPictureRecord::drawPosTextHImpl(void const
> > > +   4.00%  bench_record  bench_record                [.]
> > > SkPictureRecord::drawPosTextH(void const*, u
> > > +   3.65%  bench_record  libgcc_s.so.1               [.] __popcountdi2
> > > +   2.76%  bench_record  libc-2.15.so                [.] _int_malloc
> > > +   2.23%  bench_record  libGL.so.319.32             [.] 0xabecd
> > > +   2.20%  bench_record  bench_record                [.]
> SkPtrSet::add(void*)
> > > +   1.83%  bench_record  bench_record                [.]
> > > chromium_jpeg_idct_islow
> > > +   1.83%  bench_record  libc-2.15.so                [.] __memcpy_sse2
> > > +   1.72%  bench_record  libc-2.15.so                [.] __memcmp_sse4_1
> > > +   1.65%  bench_record  libnvidia-tls.so.319.32     [.] 0x1f6c
> > > +   1.44%  bench_record  bench_record                [.]
> > > SkWriter32::growToAtLeast(unsigned long)
> > > +   1.42%  bench_record  libz.so.1.2.3.4             [.] inflate_fast
> > > +   1.23%  bench_record  libpng12.so.0.46.0          [.] 0xbeea
> > > +   1.17%  bench_record  bench_record                [.]
> > > SkPictureRecord::drawPath(SkPath const&, SkP
> > > +   1.14%  bench_record  bench_record                [.]
> > > SkGlyphCache::VisitCache(SkTypeface*, SkDesc
> > > +   1.11%  bench_record  bench_record                [.]
> > > SkPictureRecord::drawBitmapRectToRect(SkBitm
> > > +   1.01%  bench_record  bench_record                [.]
> > > SkWriteBuffer::writeTypeface(SkTypeface*)
> > > +   1.01%  bench_record  bench_record                [.]
> > > SkClipStack::Element::updateBoundAndGenID(Sk
> > > 
> > > If I turn off inlining with compiler arguments I see a lot of time in:
> > > 
> > > SkWriter32::reserve() - so hopefully Ian's patches will give us more wins
> > there
> > > SkFlatData::operator==()
> > > SkTDynamicHash<SkFlatData>::find()
> > > SkChecksum::Compute()
> > > SkChecksum::Mash()
> > > SkWriter32::write32()
> > > SkReader32::readInt()
> > > SkFlatDictionary<SkPaint>::resetScratch()
> > > SkPictureRecord::addDraw()
> > > SkTDynamicHash<SkFlatData>::nextIndex()
> > > 
> > > (Ordering not stable across runs, so no particular order.) At least some
of
> > this
> > > is performance-loss-due-to-lack-of-inlining, rather than performance
issues
> > that
> > > were hidden by inlining. I guess Ian's focused hackery of manually
> deinlining
> > > suspect functions might be necessary to get clear signal.
> > 
> > OK, spent the morning polishing the CL.  Looks presentable to me now.
> > 
> > I reran bench_record on my iMac.  Same general results: relative runtimes in
> the
> > 80-110% range, though now with weird outliers at 67% and 170%.
> > 0.67  tabl_cnn.skp                      0.82    0.55
> > 0.79  desk_yahoosports.skp              0.0056  0.0044
> > 0.84  desk_samoasvg.skp                 0.31    0.26
> > 0.88  desk_youtubetvvideo.skp           0.026   0.023
> > 0.9   desk_googlehome.skp               0.049   0.044
> > 0.91  desk_chalkboard.skp               0.34    0.31
> > 0.92  desk_fontwipe.skp                 0.013   0.012
> > 0.92  tabl_gamedeksiam.skp              0.59    0.54
> > 0.92  tabl_slashdot.skp                 0.13    0.12
> > 0.93  desk_forecastio.skp               0.15    0.14
> > 0.93  tabl_techmeme.skp                 0.092   0.086
> > 0.94  desk_youtubetvbrowse.skp          0.018   0.017
> > 0.95  desk_gmailthread.skp              0.22    0.21
> > 0.95  desk_yahoogames.skp               0.039   0.037
> > 0.95  mobi_wikipedia.skp                0.87    0.83
> > 0.96  desk_blogger.skp                  0.23    0.22
> > 0.96  desk_carsvg.skp                   0.45    0.43
> > 0.97  desk_wordpress.skp                0.39    0.38
> > 0.97  tabl_mercurynews.skp              0.3     0.29
> > 0.98  desk_pokemonwiki.skp              5.1     5
> > 0.98  desk_tigersvg.skp                 0.048   0.047
> > 0.98  tabl_cnet.skp                     0.5     0.49
> > 0.98  tabl_gmail.skp                    0.042   0.041
> > 0.99  tabl_nofolo.skp                   0.068   0.067
> > 1     NULL                              0.01    0.01
> > 1     desk_baidu.skp                    0.12    0.12
> > 1     desk_booking.skp                  0.62    0.62
> > 1     desk_css3gradients.skp            0.25    0.25
> > 1     desk_espn.skp                     0.28    0.29
> > 1     desk_facebook.skp                 0.22    0.22
> > 1     desk_googlespreadsheet.skp        0.17    0.17
> > 1     desk_googlespreadsheetdashed.skp  0.54    0.56
> > 1     desk_gws.skp                      0.16    0.16
> > 1     desk_linkedin.skp                 0.17    0.17
> > 1     desk_mobilenews.skp               0.33    0.33
> > 1     desk_oldinboxapp.skp              0.034   0.034
> > 1     desk_pinterest.skp                0.048   0.048
> > 1     desk_rectangletransition.skp      0.02    0.02
> > 1     desk_sfgate.skp                   0.3     0.31
> > 1     desk_techcrunch.skp               0.32    0.32
> > 1     desk_twitter.skp                  0.29    0.29
> > 1     desk_weather.skp                  0.21    0.22
> > 1     desk_wowwiki.skp                  1.3     1.3
> > 1     desk_youtube.skp                  0.24    0.24
> > 1     tabl_androidpolice.skp            0.98    1
> > 1     tabl_culturalsolutions.skp        0.32    0.32
> > 1     tabl_cuteoverload.skp             0.51    0.52
> > 1     tabl_deviantart.skp               0.18    0.18
> > 1     tabl_digg.skp                     0.4     0.4
> > 1     tabl_frantzen.skp                 0.078   0.079
> > 1     tabl_googleblog.skp               0.32    0.32
> > 1     tabl_googlecalendar.skp           0.17    0.17
> > 1     tabl_gspro.skp                    0.09    0.092
> > 1     tabl_hsfi.skp                     0.39    0.39
> > 1     tabl_mlb.skp                      0.18    0.18
> > 1     tabl_mozilla.skp                  2.8     2.9
> > 1     tabl_sahadan.skp                  0.12    0.12
> > 1     tabl_ukwsj.skp                    0.41    0.42
> > 1     tabl_vnexpress.skp                0.34    0.35
> > 1     tabl_worldjournal.skp             0.39    0.4
> > 1.1   desk_amazon.skp                   0.11    0.12
> > 1.1   desk_googleplus.skp               0.33    0.35
> > 1.1   desk_jsfiddlebigcar.skp           0.039   0.041
> > 1.1   desk_jsfiddlehumperclip.skp       0.047   0.053
> > 1.1   desk_mapsvg.skp                   0.51    0.54
> > 1.1   desk_yahooanswers.skp             0.15    0.16
> > 1.1   tabl_engadget.skp                 0.41    0.44
> > 1.1   tabl_nytimes.skp                  0.28    0.31
> > 1.1   tabl_onlinewsj.skp                0.44    0.47
> > 1.1   tabl_pravda.skp                   0.23    0.25
> > 1.7   desk_ebay.skp                     0.23    0.4
> > 
> > I'm seeing qualitatively the same profile as Tom:
> >   1) memmove (copying text)
> >   2) draw (tool overhead)
> >   3) memcmp (hash table final check)
> >   4) drawPosTextHImpl (caching text metrics for playback)
> >   5) flatten
> >   6) SkPtrSet::add (flatten typefaces)
> >   7) findAndReturnMutableFlat (hash table lookup)
> >   8) something in libz (tool overhead)
> >   9) realloc
> 
> Hmm.  Reran those outliers.  They both look like noise.  I'm seeing desk_ebay
as
> ~0.24 and table_cnn as ~0.76 reliably on both here and on a clean branch.

Any thoughts guys?

[tomhudson, 2014-02-17 16:25:01]

I'm ready to run it up the flagpole and see who salutes.

I find it a bit surprising that this change offsers as large a speedup as
Dominik's http://crrev.com/158603003, but I presume that just means we don't
understand the performance Dominik's code yet.

LGTM

https://codereview.chromium.org/158913005/diff/220001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/158913005/diff/220001/src/core/SkPaint.cpp#ne...
src/core/SkPaint.cpp:2633: SkScalar* f32 = reinterpret_cast<SkScalar*>(u32);
Wicked :)

https://codereview.chromium.org/158913005/diff/220001/tests/PaintTest.cpp
File tests/PaintTest.cpp (right):

https://codereview.chromium.org/158913005/diff/220001/tests/PaintTest.cpp#new...
tests/PaintTest.cpp:263: paint.setTextScaleX(1.0f);  // Encoded despite being
the default value.
Nit: I take your comment to mean "Even though this is the the default value,
since we're explicitly setting it, it's going to get marked as dirty and we're
going to explicitly serialize it." Do we really want it to be invariant that the
class behaves this way? Or is this just an irrelevant implementation detail? Why
does it matter for correctness?

https://codereview.chromium.org/158913005/diff/220001/tests/PaintTest.cpp#new...
tests/PaintTest.cpp:270: ASSERT(writer.bytesWritten() == 48);
Hmm, == or <=?

[tomhudson, 2014-02-17 16:25:39]

The CQ bit was checked by tomhudson@google.com

[commit-bot: I haz the power, 2014-02-17 16:25:41]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/158913005/220001

[commit-bot: I haz the power, 2014-02-17 16:25:45]

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

[commit-bot: I haz the power, 2014-02-17 16:25:45]

Presubmit check for 158913005-220001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Since the CL is editing public API, you must have an LGTM from one of:
('reed@chromium.org', 'reed@google.com', 'bsalomon@chromium.org',
'bsalomon@google.com')

[tomhudson, 2014-02-17 16:27:35]

+Mike to make the bot happy.

There are changes in the SkPaint header file, but they're not to the public
function signature.

[reed1, 2014-02-17 16:35:30]

https://codereview.chromium.org/158913005/diff/220001/include/core/SkPaint.h
File include/core/SkPaint.h (right):

https://codereview.chromium.org/158913005/diff/220001/include/core/SkPaint.h#...
include/core/SkPaint.h:984: static void flatten(SkWriteBuffer& buffer, const
SkPaint& paint);
Seems like these *are* public. Do they have to be?

nit: Capitalize Static Methods

https://codereview.chromium.org/158913005/diff/220001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/158913005/diff/220001/src/core/SkPaint.cpp#ne...
src/core/SkPaint.cpp:39: kColor_Bit               = 1 <<  0,
Do these mean dirtyBits? Should they be named that way?

[mtklein_C, 2014-02-18 15:49:32]

PTAL

Rebased onto https://codereview.chromium.org/166193006/.

https://codereview.chromium.org/158913005/diff/220001/include/core/SkPaint.h
File include/core/SkPaint.h (right):

https://codereview.chromium.org/158913005/diff/220001/include/core/SkPaint.h#...
include/core/SkPaint.h:984: static void flatten(SkWriteBuffer& buffer, const
SkPaint& paint);
On 2014/02/17 16:35:31, reed1 wrote:
> Seems like these *are* public. Do they have to be?
> 
> nit: Capitalize Static Methods

These need to be accessible in SkPaintDictionary.
(Nit done.)

https://codereview.chromium.org/158913005/diff/220001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/158913005/diff/220001/src/core/SkPaint.cpp#ne...
src/core/SkPaint.cpp:39: kColor_Bit               = 1 <<  0,
On 2014/02/17 16:35:31, reed1 wrote:
> Do these mean dirtyBits? Should they be named that way?

I don't have a strong preference.  Changed.  They're bits, used to mark
dirtiness here, but dirtiness isn't necessarily the only thing they can be used
for.

https://codereview.chromium.org/158913005/diff/220001/tests/PaintTest.cpp
File tests/PaintTest.cpp (right):

https://codereview.chromium.org/158913005/diff/220001/tests/PaintTest.cpp#new...
tests/PaintTest.cpp:263: paint.setTextScaleX(1.0f);  // Encoded despite being
the default value.
On 2014/02/17 16:25:01, tomhudson wrote:
> Nit: I take your comment to mean "Even though this is the the default value,
> since we're explicitly setting it, it's going to get marked as dirty and we're
> going to explicitly serialize it." Do we really want it to be invariant that
the
> class behaves this way? Or is this just an irrelevant implementation detail?
Why
> does it matter for correctness?

Implementation detail.  Just a white-box test.

https://codereview.chromium.org/158913005/diff/220001/tests/PaintTest.cpp#new...
tests/PaintTest.cpp:270: ASSERT(writer.bytesWritten() == 48);
On 2014/02/17 16:25:01, tomhudson wrote:
> Hmm, == or <=?

Let's leave it ==.  We can change the value as the implementation changes.

[tomhudson, 2014-02-18 15:56:47]

lgtm

[reed1, 2014-02-18 16:03:42]

Looks like more trickiness to keep in sync. Hope it pays off.

lgtm

Do we have self-checks (e.g. validate()) that ensures in some way that dirtyBIts
always are correct?

[mtklein, 2014-02-18 16:08:12]

On 2014/02/18 16:03:42, reed1 wrote:
> Looks like more trickiness to keep in sync. Hope it pays off.
> 
> lgtm
> 
> Do we have self-checks (e.g. validate()) that ensures in some way that
dirtyBIts
> always are correct?

Yes, Unflatten() SkASSERTs the dirty bits roundtrip correctly.  And as usual we
have the large mass of SkPicture roundtrip checks from GMs.

[mtklein, 2014-02-18 16:19:24]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-02-18 16:19:33]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/158913005/530001

[commit-bot: I haz the power, 2014-02-18 17:25:29]

Change committed as 13487

[scroggo, 2014-02-18 18:40:54]

A revert of this CL has been created in
https://codereview.chromium.org/169183003/ by scroggo@google.com.

The reason for reverting is: Breaking the build - see
https://code.google.com/p/skia/issues/detail?id=2190.

[Sami, 2014-02-19 12:35:18]

https://codereview.chromium.org/158913005/diff/690001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/158913005/diff/690001/src/core/SkPaint.cpp#ne...
src/core/SkPaint.cpp:2610: inline static unsigned popcount(uint8_t x) {
Drive by comment: how about conditionally using gcc's __builtin_popcount for
this?

[mtklein, 2014-02-19 13:40:37]

https://codereview.chromium.org/158913005/diff/690001/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.chromium.org/158913005/diff/690001/src/core/SkPaint.cpp#ne...
src/core/SkPaint.cpp:2610: inline static unsigned popcount(uint8_t x) {
On 2014/02/19 12:35:18, Sami wrote:
> Drive by comment: how about conditionally using gcc's __builtin_popcount for
> this?

Ah, great question.  I actually started with that.  It turns out to be slower
than this, mostly because the smallest popcount GCC provides is 32 bit and we
can get away here with just counting 8.  __builtin_popcount expanded to roughly
the same logic you see here, except with two extra layers in the tree.

[mtklein, 2014-02-19 13:40:50]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-02-19 13:41:02]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/158913005/690001

[commit-bot: I haz the power, 2014-02-19 13:55:10]

Change committed as 13496

[robertphillips, 2014-02-20 13:09:40]

This was reverted in r13509 due to memory leaks (that were blocking the DEPS
roll).

[mtklein, 2014-02-21 16:04:32]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-02-21 16:04:38]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/158913005/910001

[commit-bot: I haz the power, 2014-02-21 18:18:08]

Change committed as 13536