Trace contention for lockPixels mutex

Trace contention for lockPixels mutex

Committed: https://skia.googlesource.com/skia/+/54b666b144e1281c0edf499e5ec1c59edaef6572

[enne (OOO), 2015-04-17 18:45:38]

enne@chromium.org changed reviewers:
+ reed@google.com, scroggo@google.com

[enne (OOO), 2015-04-17 18:46:10]

What do y'all think about this?

[scroggo, 2015-04-17 18:53:23]

On 2015/04/17 18:46:10, enne wrote:
> What do y'all think about this?

lgtm.

[enne (OOO), 2015-04-17 19:36:49]

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2015-04-17 19:37:01]

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

[commit-bot: I haz the power, 2015-04-17 19:42:54]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/54b666b144e1281c0edf499e5ec1c59edaef6572

[reed2, 2015-04-18 01:46:34]

reed@chromium.org changed reviewers:
+ reed@chromium.org

[reed2, 2015-04-18 01:46:35]

Is there a bug or experiment that needs this? Should there be a comment in the
code describing why we're bracketing this particular mutex lock?

[enne (OOO), 2015-04-20 18:30:47]

On 2015/04/18 at 01:46:35, reed wrote:
> Is there a bug or experiment that needs this? Should there be a comment in the
code describing why we're bracketing this particular mutex lock?

I had a meeting to brainstorm what could be improved in Chrome to better
understand image decoding/scaling/caching performance.  I've filed that here:
https://code.google.com/p/chromium/issues/detail?id=478824.  Hopefully that
provides some better context.

I'd like to add this trace as something that telemetry tracks for image decoding
tests.  David Reveman did an experiment where he turned off the ImageDecodeTask
in Chrome, and (along with frame times) this trace made it very clear how much
time was being spent in raster threads waiting on other raster threads to finish
their decodes.

I did not leave a comment in the code because it seemed pretty clear that a
trace around a mutex is trying to make visible contention for that mutex. My
experience in Chrome is that traces generally don't get comments.  If you think
a comment would make this better, I would be happy to do it.

[tomhudson, 2015-04-21 11:28:00]

This is a good idea for local testing or telemetry runs, but a couple of years
ago we had real performance problems on Chrome for Android whenever fine-grained
trace-events (firing hundreds of times per frame) were landed. How frequently is
this going to be hit? Do we have a sense of its impact on Clank performance? (I
don't know if we have any Skia tests that run with tracing enabled. Since users
won't usually be tracing perhaps this isn't a concern.)

[enne (OOO), 2015-04-22 21:07:20]

In practice, there would be one trace per image per raster task.  In practice,
as the number of images scales, the cache starts failing, and traces start being
very small next to image decodes.  So, I hear your concerns, but I am not too
worried about this.