Adding performance tracking for canvas API calls

Adding performance tracking for canvas API calls

In this CL, we add timer to some of the expensive API calls in 2d canvas.
These API calls includes:
1. drawImage
2. createImageData
3. getImageData
4. putImageData
5. toDataURL

Measuring toBlob will be in another CL, it is more complicated than this
because of its idle scheduling scheme.

BUG=612585
Committed: https://crrev.com/5aa1628e82232fab9951a2895094a216661271f4
Cr-Commit-Position: refs/heads/master@{#396105}

[xidachen, 2016-05-19 14:51:08]

xidachen@chromium.org changed reviewers:
+ junov@chromium.org

[xidachen, 2016-05-19 14:51:09]

PTAL

[Justin Novosad, 2016-05-19 15:28:40]

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:609:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterGPU,
("Blink.Canvas.ToDataURL.GPU", 0, 10000000, 50));
You are missing entries in histograms.xml

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:612:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterDL,
("Blink.Canvas.ToDataURL.DL", 0, 10000000, 50));
DL -> DisplayList

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:615:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterSW,
("Blink.Canvas.ToDataURL.SW", 0, 10000000, 50));
SW -> CPU

[Justin Novosad, 2016-05-19 15:36:56]

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:620:
exceptionState.throwSecurityError("Tainted canvases may not be exported.");
We should start the measurement after this early exit condition

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:630: return
toDataURLInternal(mimeType, quality, BackBuffer);
Specifically for toDataURL, it would make more sense to classify by mimeType
rather than by rendering mode (GPU/CPU/DisplayList).

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:991:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterGPU,
("Blink.Canvas.DrawImage.GPU", 0, 10000000, 50));
Should also classify by type of image source.

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1185:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterGPU,
("Blink.Canvas.CreateImageData.GPU", 0, 10000000, 50));
I don think we need to measure this one. It's overkill.

[xidachen, 2016-05-20 12:36:53]

xidachen@chromium.org changed reviewers:
+ isherman@chromium.org

[xidachen, 2016-05-20 12:36:54]

+isherman@: histogram owner

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:612:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterDL,
("Blink.Canvas.ToDataURL.DL", 0, 10000000, 50));
On 2016/05/19 15:28:39, Justin Novosad wrote:
> DL -> DisplayList

Changed in all places.

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:615:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterSW,
("Blink.Canvas.ToDataURL.SW", 0, 10000000, 50));
On 2016/05/19 15:28:40, Justin Novosad wrote:
> SW -> CPU

Changed in all places.

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:620:
exceptionState.throwSecurityError("Tainted canvases may not be exported.");
On 2016/05/19 15:36:55, Justin Novosad wrote:
> We should start the measurement after this early exit condition

Done.

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:630: return
toDataURLInternal(mimeType, quality, BackBuffer);
On 2016/05/19 15:36:55, Justin Novosad wrote:
> Specifically for toDataURL, it would make more sense to classify by mimeType
> rather than by rendering mode (GPU/CPU/DisplayList).

Done.

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:991:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterGPU,
("Blink.Canvas.DrawImage.GPU", 0, 10000000, 50));
On 2016/05/19 15:36:55, Justin Novosad wrote:
> Should also classify by type of image source.

Done.

https://codereview.chromium.org/1992253002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1185:
DEFINE_STATIC_LOCAL(CustomCountHistogram, scopedUsCounterGPU,
("Blink.Canvas.CreateImageData.GPU", 0, 10000000, 50));
On 2016/05/19 15:36:55, Justin Novosad wrote:
> I don think we need to measure this one. It's overkill.

Done.

[Ilya Sherman, 2016-05-20 21:54:22]

Please use histogram_suffixes elements to reduce the amount of repetition in the
histograms.xml file.  Note that histogram_suffixes can stack.

[xidachen, 2016-05-24 14:35:54]

Changed to use histogram_suffixes, PTAL.

[Justin Novosad, 2016-05-24 14:47:35]

On 2016/05/24 14:35:54, xidachen wrote:
> Changed to use histogram_suffixes, PTAL.

The expansive instrumentation is unfortunate, but I understand why it is
necessary and I have nothing better to suggest.

WebKit/Source lgtm

[Ilya Sherman, 2016-05-25 00:20:04]

LGTM % a nit.  Since you're adding quite a large number of histograms, I'd
recommend filing at least a mental TODO to come back to these and prune off any
that you find aren't needed, once you've had a chance to look at the data.

https://codereview.chromium.org/1992253002/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1992253002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:90998: +</histogram_suffixes>
You could condense these down to just:

<histogram_suffixes name="BlinkCanvasDrawImageType" separator=".">
  <suffix name="Canvas"/>
  <suffix name="ImageBitmap"/>
  <suffix name="Others"/>
  <suffix name="SVG"/>
  <suffix name="Video"/>
  <affected-histogram name="Blink.Canvas.DrawImage"/>
</histogram_suffixes>

<histogram_suffixes name="BlinkCanvasDurationBySource" separator=".">
  <suffix name="CPU"/>
  <suffix name="DisplayList"/>
  <suffix name="GPU"/>
  <affected-histogram name="Blink.Canvas.DrawImage.Canvas"/>
  <affected-histogram name="Blink.Canvas.DrawImage.ImageBitmap"/>
  <affected-histogram name="Blink.Canvas.DrawImage.Others"/>
  <affected-histogram name="Blink.Canvas.DrawImage.SVG"/>
  <affected-histogram name="Blink.Canvas.DrawImage.Video"/>
  <affected-histogram name="Blink.Canvas.GetImageData"/>
  <affected-histogram name="Blink.Canvas.PutImageData"/>
</histogram_suffixes>

(Note that you'd need to name the DrawImage histograms with an extra dot, e.g.
"Blink.Canvas.DrawImage.Canvas.GPU" instead of
"Blink.Canvas.DrawImage.CanvasGPU".  But I think that's preferable anyway.)

[xidachen, 2016-05-25 01:35:06]

https://codereview.chromium.org/1992253002/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1992253002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:90998: +</histogram_suffixes>
On 2016/05/25 00:20:04, Ilya Sherman wrote:
> You could condense these down to just:
> 
> <histogram_suffixes name="BlinkCanvasDrawImageType" separator=".">
>   <suffix name="Canvas"/>
>   <suffix name="ImageBitmap"/>
>   <suffix name="Others"/>
>   <suffix name="SVG"/>
>   <suffix name="Video"/>
>   <affected-histogram name="Blink.Canvas.DrawImage"/>
> </histogram_suffixes>
> 
> <histogram_suffixes name="BlinkCanvasDurationBySource" separator=".">
>   <suffix name="CPU"/>
>   <suffix name="DisplayList"/>
>   <suffix name="GPU"/>
>   <affected-histogram name="Blink.Canvas.DrawImage.Canvas"/>
>   <affected-histogram name="Blink.Canvas.DrawImage.ImageBitmap"/>
>   <affected-histogram name="Blink.Canvas.DrawImage.Others"/>
>   <affected-histogram name="Blink.Canvas.DrawImage.SVG"/>
>   <affected-histogram name="Blink.Canvas.DrawImage.Video"/>
>   <affected-histogram name="Blink.Canvas.GetImageData"/>
>   <affected-histogram name="Blink.Canvas.PutImageData"/>
> </histogram_suffixes>
> 
> (Note that you'd need to name the DrawImage histograms with an extra dot, e.g.
> "Blink.Canvas.DrawImage.Canvas.GPU" instead of
> "Blink.Canvas.DrawImage.CanvasGPU".  But I think that's preferable anyway.)

Thank you, this is much cleaner.

[xidachen, 2016-05-25 01:35:26]

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

[commit-bot: I haz the power, 2016-05-25 01:36:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992253002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992253002/80001

[commit-bot: I haz the power, 2016-05-25 03:08:58]

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

[commit-bot: I haz the power, 2016-05-25 03:08:59]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xidachen, 2016-05-25 14:46:06]

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

[commit-bot: I haz the power, 2016-05-25 14:46:23]

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

[commit-bot: I haz the power, 2016-05-25 19:10:19]

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

[commit-bot: I haz the power, 2016-05-25 19:10:20]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xidachen, 2016-05-25 23:32:40]

The CQ bit was checked by xidachen@chromium.org

[xidachen, 2016-05-25 23:32:41]

The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org,
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1992253002/#ps100001
(title: "use thread safe static local")

[commit-bot: I haz the power, 2016-05-25 23:33:53]

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

[commit-bot: I haz the power, 2016-05-26 03:40:50]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-26 03:42:57]

Description was changed from

==========
Adding performance tracking for canvas API calls

In this CL, we add timer to some of the expensive API calls in 2d canvas.
These API calls includes:
1. drawImage
2. createImageData
3. getImageData
4. putImageData
5. toDataURL

Measuring toBlob will be in another CL, it is more complicated than this
because of its idle scheduling scheme.

BUG=612585
==========

to

==========
Adding performance tracking for canvas API calls

In this CL, we add timer to some of the expensive API calls in 2d canvas.
These API calls includes:
1. drawImage
2. createImageData
3. getImageData
4. putImageData
5. toDataURL

Measuring toBlob will be in another CL, it is more complicated than this
because of its idle scheduling scheme.

BUG=612585

Committed: https://crrev.com/5aa1628e82232fab9951a2895094a216661271f4
Cr-Commit-Position: refs/heads/master@{#396105}
==========

[commit-bot: I haz the power, 2016-05-26 03:42:59]

Patchset 6 (id:??) landed as
https://crrev.com/5aa1628e82232fab9951a2895094a216661271f4
Cr-Commit-Position: refs/heads/master@{#396105}