History log of /external/skia/src/core/SkRecord.h
Revision Date Author Comments (<<< Hide modified files) (Show modified files >>>)
29b1afc169576cf5e708e46b74313b5666e66249 09-Apr-2015 mtklein <mtklein@chromium.org> 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

Review URL: https://codereview.chromium.org/1061783002
/external/skia/src/core/SkRecord.h
35f55764b81390a085fb90f624082c196fbd6229 08-Apr-2015 mtklein <mtklein@google.com> Revert of Rearrange SkRecord with small N in mind (patchset #8 id:120001 of https://codereview.chromium.org/1061783002/)

Reason for revert:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug/builds/149/steps/dm/logs/stdio

Original issue's description:
> 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

TBR=reed@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:470553

Review URL: https://codereview.chromium.org/1068383003
/external/skia/src/core/SkRecord.h
e2dd9408cd711777afaa9410427fb0d761ab004a 08-Apr-2015 mtklein <mtklein@chromium.org> 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

Review URL: https://codereview.chromium.org/1061783002
/external/skia/src/core/SkRecord.h
59dba146fe4170c4986b2494414c08be2c93d4a2 12-Dec-2014 mtklein <mtklein@chromium.org> SkRecord: increase min block to 512B, remove max.

When we added the 64K allocation cap, the bots showed we took a perf hit
on some large .skps like desk_pokemonwiki.skp, despite not seeing a local
effect. I'm still not seeing that locally, but I'd like to try removing the cap on
the bots to see what happens. For big monolithic pictures, really packing into
memory tightly is probably not as important as it is for tiny ones.

Similarly, we're probably being too cautious about making tiny allocations.
Today we start at 16 bytes, which isn't really enough to record anything.
Even the smallest picture, say,
save
clipRect
drawRect
restore
requires ~200 bytes, so we might as well move our minimum block size up
near there.

I don't know if 16 bytes is too small to start for GrTextStrikes, so I've left the
behavior the same (though the max is still gone).

Local recording performance is neutral-to-positive:
tabl_deviantart.skp 126us -> 129us 1.02x
tabl_nytimes.skp 110us -> 112us 1.02x
tabl_cuteoverload.skp 521us -> 530us 1.02x
desk_mobilenews.skp 673us -> 682us 1.01x
desk_chalkboard.skp 843us -> 854us 1.01x
desk_sfgate.skp 528us -> 535us 1.01x
desk_silkfinance.skp 68.2us -> 69us 1.01x
desk_youtube.skp 623us -> 629us 1.01x
desk_blogger.skp 472us -> 475us 1.01x
desk_jsfiddlehumperclip.skp 42.2us -> 42.5us 1.01x
desk_espn.skp 255us -> 256us 1.01x
desk_ebay.skp 174us -> 174us 1x
desk_twitter.skp 454us -> 455us 1x
tabl_pravda.skp 200us -> 201us 1x
desk_wordpress.skp 782us -> 784us 1x
desk_samoasvg.skp 762us -> 761us 1x
tabl_mozilla.skp 1.58ms -> 1.58ms 1x
tabl_slashdot.skp 107us -> 107us 1x
tabl_techmeme.skp 102us -> 102us 0.99x
tabl_gamedeksiam.skp 729us -> 724us 0.99x
tabl_nofolo.skp 65.3us -> 64.7us 0.99x
desk_gmailthread.skp 339us -> 336us 0.99x
tabl_sahadan.skp 91us -> 90us 0.99x
desk_yahooanswers.skp 144us -> 142us 0.99x
tabl_cnet.skp 143us -> 141us 0.99x
tabl_googleblog.skp 206us -> 203us 0.99x
tabl_cnn.skp 160us -> 158us 0.99x
tabl_frantzen.skp 50.5us -> 49.6us 0.98x
desk_linkedin.skp 328us -> 323us 0.98x
tabl_digg.skp 790us -> 769us 0.97x
desk_jsfiddlebigcar.skp 40.6us -> 39.5us 0.97x
desk_mapsvg.skp 1.57ms -> 1.52ms 0.97x
tabl_gmail.skp 19.4us -> 18.6us 0.96x
tabl_hsfi.skp 9.81us -> 9.11us 0.93x

BUG=skia:

Review URL: https://codereview.chromium.org/793033002
/external/skia/src/core/SkRecord.h
f98862c39b7e79a35f1907cb94240f5d0545fe7b 24-Nov-2014 mtklein <mtklein@chromium.org> SkRecord: outline methods that are not called O(N) times.

Looks like a noop-to-minor-win:
tabl_sahadan.skp 94.9us -> 98.6us 1x
desk_jsfiddlebigcar.skp 38.9us -> 39.7us 1x
desk_silkfinance.skp 78us -> 78.9us 1x
desk_jsfiddlehumperclip.skp 43.8us -> 44.3us 1x
desk_sfgate.skp 547us -> 548us 1x
tabl_gmail.skp 19.9us -> 19.8us 1x
tabl_worldjournal.skp 230us -> 229us 1x
desk_css3gradients.skp 248us -> 247us 1x
tabl_cnn.skp 205us -> 203us 0.99x
desk_linkedin.skp 342us -> 340us 0.99x
desk_wowwiki.skp 1.63ms -> 1.62ms 0.99x
tabl_cnet.skp 142us -> 141us 0.99x
desk_pokemonwiki.skp 9.76ms -> 9.67ms 0.99x
desk_espn.skp 267us -> 264us 0.99x
desk_youtube.skp 576us -> 570us 0.99x
tabl_pravda.skp 238us -> 235us 0.99x
tabl_ukwsj.skp 566us -> 560us 0.99x
tabl_engadget.skp 630us -> 622us 0.99x
desk_googlespreadsheetdashed.skp 1.66ms -> 1.64ms 0.99x
desk_mobilenews.skp 486us -> 480us 0.99x
tabl_googlecalendar.skp 211us -> 208us 0.99x
desk_samoasvg.skp 740us -> 730us 0.99x
desk_gws.skp 187us -> 184us 0.99x
desk_ebay.skp 234us -> 230us 0.99x
desk_mapsvg.skp 1.6ms -> 1.58ms 0.98x
tabl_nytimes.skp 130us -> 128us 0.98x
tabl_googleblog.skp 305us -> 300us 0.98x
desk_fontwipe.skp 40.3us -> 39.6us 0.98x
desk_tigersvg.skp 189us -> 186us 0.98x
tabl_androidpolice.skp 662us -> 650us 0.98x
desk_wordpress.skp 824us -> 809us 0.98x
tabl_mlb.skp 338us -> 331us 0.98x
tabl_culturalsolutions.skp 390us -> 382us 0.98x
desk_baidu.skp 213us -> 208us 0.98x
tabl_gspro.skp 72.9us -> 71.1us 0.97x
tabl_nofolo.skp 74us -> 71.9us 0.97x
desk_yahooanswers.skp 173us -> 168us 0.97x
tabl_frantzen.skp 57.3us -> 55.6us 0.97x
desk_chalkboard.skp 891us -> 865us 0.97x
desk_pinterest.skp 154us -> 149us 0.97x
desk_blogger.skp 537us -> 519us 0.97x
tabl_hsfi.skp 10.1us -> 9.69us 0.96x
desk_gmailthread.skp 333us -> 318us 0.96x
tabl_digg.skp 926us -> 883us 0.95x
desk_googlespreadsheet.skp 586us -> 558us 0.95x
desk_forecastio.skp 101us -> 95.7us 0.95x
desk_booking.skp 1.1ms -> 1.04ms 0.95x
tabl_deviantart.skp 144us -> 136us 0.95x
desk_facebook.skp 584us -> 553us 0.95x
desk_weather.skp 289us -> 272us 0.94x
desk_googlehome.skp 61.1us -> 57.5us 0.94x
desk_googleplus.skp 914us -> 849us 0.93x
desk_twitter.skp 499us -> 463us 0.93x

BUG=skia:

Review URL: https://codereview.chromium.org/756783002
/external/skia/src/core/SkRecord.h
1bdfd3f4f09e47364f76d3f08177b1ce844ac786 24-Nov-2014 reed <reed@google.com> option to return drawable from recording

patch from issue 747033005 at patchset 80001 (http://crrev.com/747033005#ps80001)

BUG=skia:

Review URL: https://codereview.chromium.org/732653004
/external/skia/src/core/SkRecord.h
08d1fccf6eeec0a9fd5421e59e4d05daccf6e339 20-Nov-2014 mtklein <mtklein@chromium.org> Add SkNVRefCnt, prune down SkPicture's size

SkNVRefCnt is a variant of SkRefCnt that's Not Virtual, so weighs 4 bytes
instead of 8 or 16. There's only benefit to doing this if the deriving class
does not otherwise need a vtable, e.g. SkPicture.

I've stripped out some cruft from SkPicture, rearranged fields to pack tightly,
and added compile asserts for the sizes of SkPicture, SkRecord, and
SkVarAlloc.

BUG=skia:3144

Review URL: https://codereview.chromium.org/741793002
/external/skia/src/core/SkRecord.h
158fcaa031d105dc999d9813fee8927db56a871c 19-Nov-2014 tomhudson <tomhudson@chromium.org> Implement SkPicture::bytesUsed() for SkRecord backend

BUG=chromium:230419
R=mtklein@google.com,reed@google.com

Review URL: https://codereview.chromium.org/490253003
/external/skia/src/core/SkRecord.h
f2950b1c4538fa638a594af6beacd3d1434eb74d 13-Nov-2014 mtklein <mtklein@chromium.org> Deparameterize SkVarAlloc.

SkRecord performance is not sensitive to these values, so we can cut some
storage. This rearranges the space-remaining logic to use a count of bytes
left rather than a pointer to the end, cutting memory usage a little more.

An SkVarAlloc used to weigh 20 or 32 bytes which now becomes 16 or 24.

I think if I think about it a bit more I can trim off that Block* too,
getting us to 12 or 16 bytes.

Because we now just always grow by doubling, this CL switches from storing
fSmallest to its log base 2. This has the nice effect of never having to worry
about it overflowing, and means we can probably squeeze it down into a byte
if we want, even 6 bits.

BUG=skia:

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

CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu12-ShuttleA-GTX660-x86-Debug-Trybot

Review URL: https://codereview.chromium.org/721313002
/external/skia/src/core/SkRecord.h
dfd5f6edf83ff6b42bf38f03b9633c65d80eb6cb 13-Nov-2014 mtklein <mtklein@google.com> Revert of Deparameterize SkVarAlloc. (patchset #6 id:100001 of https://codereview.chromium.org/721313002/)

Reason for revert:
Unit test failures on 32-bit machines.

test Record_Alignment: ../../tests/RecordTest.cpp:100 is_aligned(record.alloc<uint64_t>())

Original issue's description:
> Deparameterize SkVarAlloc.
>
> SkRecord performance is not sensitive to these values, so we can cut some
> storage. This rearranges the space-remaining logic to use a count of bytes
> left rather than a pointer to the end, cutting memory usage a little more.
>
> An SkVarAlloc used to weigh 20 or 32 bytes which now becomes 16 or 24.
>
> I think if I think about it a bit more I can trim off that Block* too,
> getting us to 12 or 16 bytes.
>
> Because we now just always grow by doubling, this CL switches from storing
> fSmallest to its log base 2. This has the nice effect of never having to worry
> about it overflowing, and means we can probably squeeze it down into a byte
> if we want, even 6 bits.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/bc415389855888af5a1282ca4b6bee30afa3d69d

TBR=reed@google.com,mtklein@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/718203006
/external/skia/src/core/SkRecord.h
bc415389855888af5a1282ca4b6bee30afa3d69d 13-Nov-2014 mtklein <mtklein@chromium.org> Deparameterize SkVarAlloc.

SkRecord performance is not sensitive to these values, so we can cut some
storage. This rearranges the space-remaining logic to use a count of bytes
left rather than a pointer to the end, cutting memory usage a little more.

An SkVarAlloc used to weigh 20 or 32 bytes which now becomes 16 or 24.

I think if I think about it a bit more I can trim off that Block* too,
getting us to 12 or 16 bytes.

Because we now just always grow by doubling, this CL switches from storing
fSmallest to its log base 2. This has the nice effect of never having to worry
about it overflowing, and means we can probably squeeze it down into a byte
if we want, even 6 bits.

BUG=skia:

Review URL: https://codereview.chromium.org/721313002
/external/skia/src/core/SkRecord.h
8113dd13692bc2e1fe804141d04b2cc5f03a55be 13-Nov-2014 mtklein <mtklein@chromium.org> SkVarAlloc

Like SkChunkAlloc, but
- does its allocation with better sympathy for malloc granularity;
- the fast path inlines entirely;
- smaller per-block overhead;
- smaller per-SkVarAlloc overhead;
- growth parameters are a little more tunable.

Its main downside is less flexibility; it supports fewer methods than SkChunkAlloc.

These current parameters bring the first allocation down from 4K to 1K,
without affecting recording time on my desktop. skiaperf.com will tell the
whole story.

BUG=skia:

Review URL: https://codereview.chromium.org/674263002
/external/skia/src/core/SkRecord.h
db26a1267d57f6d8ef5b2fbdeb519be718fc1760 15-Sep-2014 mtklein <mtklein@skia.org> SkRecord: clean up comments and 4 bytes per SkRecord.

Just happened to notice these while skimming through. The comments are a
little out of date; now that we have visit/mutate, we do need O(1) random
access.

The constructor arguments are flexibility we're not using.

BUG=skia:
R=fmalita@google.com

Author: mtklein@skia.org

Review URL: https://codereview.chromium.org/569373002
/external/skia/src/core/SkRecord.h
0209e95cc2625a445c1cb6c4213d2182e5c832d7 28-Aug-2014 mtklein <mtklein@chromium.org> Align all SkRecord::alloc() calls up to at least a pointer size.

This should make the LSAN bots able to see all our pointers.

BUG=skia:
R=reed@google.com, robertphillips@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/517073002
/external/skia/src/core/SkRecord.h
2347b624678fedf1d2f7ab1d79b9ad70087c3392 07-Aug-2014 reed <reed@google.com> change drawPicture in SkRecord to just ref the picture

also fix some int/unsigned/size_t warnings

BUG=skia:
R=mtklein@google.com

Author: reed@google.com

Review URL: https://codereview.chromium.org/449933002
/external/skia/src/core/SkRecord.h
887f3979f0c717e69a8b7d169169bc27eb46d3b5 17-Jun-2014 mtklein <mtklein@chromium.org> Add EXPERIMENTAL_beginRecording() for SkRecord-based recording.

The interesting stuff is in SkPictureRecorder.{h,cpp}. The rest is mostly moving SkRecord from its own directories into core to avoid circular dependencies in GYP.

After plumbing SkRecord all the way through in Picture, I'll delete its old entry point include/record/SkRecording.h. For now it and record.gypi need to stay where they are to keep Chrome building.

BUG=skia:
R=reed@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/331573004
/external/skia/src/core/SkRecord.h