Adding performance benchmark for creating imagebitmap from imagedata

Adding performance benchmark for creating imagebitmap from imagedata

BUG=538253
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect

Committed: https://crrev.com/9f0bb323d1b080be08797b13fa77e2d6153c5800
Cr-Commit-Position: refs/heads/master@{#353785}

[xidachen, 2015-10-05 20:16:49]

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

[xidachen, 2015-10-05 20:20:32]

This is the benchmark test for createImageBitmap from ImageData. Since
createImageBitmap is still an experimental feature, you will need to create an
http server under the dir "third_party/WebKit/PerformanceTests", and
"--enable-experimental-canvas-features" is needed when running the benchmark.

Please review it.

Thank you.

[Justin Novosad, 2015-10-06 02:06:32]

On 2015/10/05 20:20:32, xidachen wrote:
> This is the benchmark test for createImageBitmap from ImageData. Since
> createImageBitmap is still an experimental feature, you will need to create an
> http server under the dir "third_party/WebKit/PerformanceTests", and
> "--enable-experimental-canvas-features" is needed when running the benchmark.
> 
> Please review it.
> 
> Thank you.

Does it run properly with the command line: 
telemetry/run_benchmark --browser=release blink_perf.canvas

This is how the perf bots will run the test.

[Justin Novosad, 2015-10-06 02:17:53]

https://codereview.chromium.org/1387933002/diff/1/third_party/WebKit/Performa...
File
third_party/WebKit/PerformanceTests/Canvas/createImageBitmapFromImageData.html
(right):

https://codereview.chromium.org/1387933002/diff/1/third_party/WebKit/Performa...
third_party/WebKit/PerformanceTests/Canvas/createImageBitmapFromImageData.html:18:
function setImageData() {
I would call this initializeImageData.

https://codereview.chromium.org/1387933002/diff/1/third_party/WebKit/Performa...
third_party/WebKit/PerformanceTests/Canvas/createImageBitmapFromImageData.html:19:
for(var i = 0; i < image.data.length; i+= 4)
if you just incremented i by 1, you would not need two nested loops.

https://codereview.chromium.org/1387933002/diff/1/third_party/WebKit/Performa...
third_party/WebKit/PerformanceTests/Canvas/createImageBitmapFromImageData.html:21:
image.data[i+j] = rand(255);
Not that it matters much, but to produce values between 0 and 255 uniformly, you
should call rand(256).  Look it up: Math.random() produces value in the range
[0,1)

https://codereview.chromium.org/1387933002/diff/1/third_party/WebKit/Performa...
third_party/WebKit/PerformanceTests/Canvas/createImageBitmapFromImageData.html:25:
createImageBitmap(image, 0, 0, imgWidth, imgHeight);
Add a comment to explain that the return Promise is not retained because this
test is meant to only measure the immediate run time of createImageBitmap from
an ImageData, which is not to be implemented in a way that does all the work
synchronously, even though the API is technically async.

[Justin Novosad, 2015-10-06 02:19:17]

> an ImageData, which is not to be implemented in a way that does all the work

typo: "not" -> "known"

[xidachen, 2015-10-06 02:26:05]

On 2015/10/06 02:06:32, Justin Novosad wrote:
> On 2015/10/05 20:20:32, xidachen wrote:
> > This is the benchmark test for createImageBitmap from ImageData. Since
> > createImageBitmap is still an experimental feature, you will need to create
an
> > http server under the dir "third_party/WebKit/PerformanceTests", and
> > "--enable-experimental-canvas-features" is needed when running the
benchmark.
> > 
> > Please review it.
> > 
> > Thank you.
> 
> Does it run properly with the command line: 
> telemetry/run_benchmark --browser=release blink_perf.canvas
> 
> This is how the perf bots will run the test.

Hi Justin,

I thought running this file directly in the browser will measure how many times
createImageBitmap() can run in 1 second. For example, if I open the
"drawimage.html" in the browser, it will show the result like "drawimage runs X
times per second". And my impression is that this is a direct measure for the
performance of this function. Please correct me if this is wrong.

Thanks.

[xidachen, 2015-10-06 13:19:15]

On 2015/10/06 02:06:32, Justin Novosad wrote:
> On 2015/10/05 20:20:32, xidachen wrote:
> > This is the benchmark test for createImageBitmap from ImageData. Since
> > createImageBitmap is still an experimental feature, you will need to create
an
> > http server under the dir "third_party/WebKit/PerformanceTests", and
> > "--enable-experimental-canvas-features" is needed when running the
benchmark.
> > 
> > Please review it.
> > 
> > Thank you.
> 
> Does it run properly with the command line: 
> telemetry/run_benchmark --browser=release blink_perf.canvas
> 
> This is how the perf bots will run the test.

Hi Justin,

Please ignore my previous reply. You can run it using this command line:
telemetry/run_benchmark --browser=release
--extra-browser-args=--enable-experimental-canvas-features blink_perf.canvas

I just successfully run that and got result.

Thank you.

[xidachen, 2015-10-06 13:47:30]

Hi Justin,

I have fixed issues with function name and added some comments. 

You can run this test using this command line:
./run_benchmark --browser=default
--extra-browser-args=--enable-experimental-canvas-features blink_perf.canvas

In my case, default is a release build.

Thank you.

[Justin Novosad, 2015-10-08 00:40:51]

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

[Justin Novosad, 2015-10-08 00:40:51]

+sullivan

Xida, that is great, but now you have to find the script that runs these tests
on the perf bot and make blink_perf.canvas runs with the
--enable-experimental-canvas-features flag.

Annie, I have no idea how these scripts work. can you point Xida in the right
direction?

[sullivan, 2015-10-08 03:02:23]

On 2015/10/08 00:40:51, Justin Novosad wrote:
> +sullivan
> 
> Xida, that is great, but now you have to find the script that runs these tests
> on the perf bot and make blink_perf.canvas runs with the
> --enable-experimental-canvas-features flag.
> 
> Annie, I have no idea how these scripts work. can you point Xida in the right
> direction?

https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...

[xidachen, 2015-10-08 12:30:21]

On 2015/10/08 00:40:51, Justin Novosad wrote:
> +sullivan
> 
> Xida, that is great, but now you have to find the script that runs these tests
> on the perf bot and make blink_perf.canvas runs with the
> --enable-experimental-canvas-features flag.
> 
> Annie, I have no idea how these scripts work. can you point Xida in the right
> direction?

Hi Justin,

I don't understand the part "run the tests on the perf bot", how should I run it
instead of using the command line.

Thank you.

[sullivan, 2015-10-08 13:39:12]

On 2015/10/08 12:30:21, xidachen wrote:
> On 2015/10/08 00:40:51, Justin Novosad wrote:
> > +sullivan
> > 
> > Xida, that is great, but now you have to find the script that runs these
tests
> > on the perf bot and make blink_perf.canvas runs with the
> > --enable-experimental-canvas-features flag.
> > 
> > Annie, I have no idea how these scripts work. can you point Xida in the
right
> > direction?
> 
> Hi Justin,
> 
> I don't understand the part "run the tests on the perf bot", how should I run
it
> instead of using the command line.
> 
> Thank you.

The perf bots run the command line:
tools/perf/run_benchmark bink_perf.canvas

[xidachen, 2015-10-08 13:56:16]

On 2015/10/08 13:39:12, sullivan wrote:
> On 2015/10/08 12:30:21, xidachen wrote:
> > On 2015/10/08 00:40:51, Justin Novosad wrote:
> > > +sullivan
> > > 
> > > Xida, that is great, but now you have to find the script that runs these
> tests
> > > on the perf bot and make blink_perf.canvas runs with the
> > > --enable-experimental-canvas-features flag.
> > > 
> > > Annie, I have no idea how these scripts work. can you point Xida in the
> right
> > > direction?
> > 
> > Hi Justin,
> > 
> > I don't understand the part "run the tests on the perf bot", how should I
run
> it
> > instead of using the command line.
> > 
> > Thank you.
> 
> The perf bots run the command line:
> tools/perf/run_benchmark bink_perf.canvas

Thanks Annie.
I tried "./tools/perf/run_benchmark --browser=default
--extra-browser-args=--enable-experimental-canvas-features blink_perf.canvas",
it works fine. The result I got for this particular benchmark is 642 +- 4.82%.
My understanding is "tools/perf/run_benchmark bink_perf.canvas" will
automatically run all the benchmark tests that are placed under the dir
"third_party/WebKit/PerformanceTests/Canvas", is that correct?

[sullivan, 2015-10-08 14:01:28]

On 2015/10/08 13:56:16, xidachen wrote:
> On 2015/10/08 13:39:12, sullivan wrote:
> > On 2015/10/08 12:30:21, xidachen wrote:
> > > On 2015/10/08 00:40:51, Justin Novosad wrote:
> > > > +sullivan
> > > > 
> > > > Xida, that is great, but now you have to find the script that runs these
> > tests
> > > > on the perf bot and make blink_perf.canvas runs with the
> > > > --enable-experimental-canvas-features flag.
> > > > 
> > > > Annie, I have no idea how these scripts work. can you point Xida in the
> > right
> > > > direction?
> > > 
> > > Hi Justin,
> > > 
> > > I don't understand the part "run the tests on the perf bot", how should I
> run
> > it
> > > instead of using the command line.
> > > 
> > > Thank you.
> > 
> > The perf bots run the command line:
> > tools/perf/run_benchmark bink_perf.canvas
> 
> Thanks Annie.
> I tried "./tools/perf/run_benchmark --browser=default
> --extra-browser-args=--enable-experimental-canvas-features blink_perf.canvas",
> it works fine. The result I got for this particular benchmark is 642 +- 4.82%.
> My understanding is "tools/perf/run_benchmark bink_perf.canvas" will
> automatically run all the benchmark tests that are placed under the dir
> "third_party/WebKit/PerformanceTests/Canvas", is that correct?

That's correct. If you want the perfbots to run with
--enable-experimental-canvas-features you need to modify the benchmark (or add a
second one with the flag)

[xidachen, 2015-10-08 15:17:57]

> That's correct. If you want the perfbots to run with
> --enable-experimental-canvas-features you need to modify the benchmark (or add
a
> second one with the flag)

Hi Annie,
When I run "tools/perf/run_benchmark blink_perf.canvas", it gives an error like
this:

CustomizeBrowserOptions at tools/perf/benchmarks/blink_perf.py:92
    if 'content-shell' in options.browser_type:
TypeError: argument of type 'NoneType' is not iterable

But if I do "tools/perf/run_benchmark --browser=default blink_perf.canvas", then
it works perfectly fine.
Is that an issue with the telemetry?

Thank you.

[Justin Novosad, 2015-10-08 16:55:17]

That is normal. the --browser arg is mandatory. The important thing is to modify
the script to make it start the browser with the command line options you need
without having to specify them on the run_benchmark command line.  This is so
that the bots run the tests with the right configuration.  IMHO, it would be
perfectly fine to enable experimental canvas features on all of blink_perf if
that is easier than targeting just blink_perf.canvas.

[xidachen, 2015-10-08 17:31:38]

The "--extra-browser-args=--enable-experimental-canvas-features" should be by
default for all blink_perf tests.

Run "tools/perf/run_benchmark --browser=default blink_perf_canvas" for this
particular test.

[Justin Novosad, 2015-10-08 17:42:20]

On 2015/10/08 17:31:38, xidachen wrote:
> The "--extra-browser-args=--enable-experimental-canvas-features" should be by
> default for all blink_perf tests.
> 
> Run "tools/perf/run_benchmark --browser=default blink_perf_canvas" for this
> particular test.

lgtm for third_party/WebKit
Still need approval from sullivan@ for blink_perf.py

[chrishtr, 2015-10-08 17:59:05]

https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
File tools/perf/benchmarks/blink_perf.py (right):

https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
tools/perf/benchmarks/blink_perf.py:90: '--enable-experimental-canvas-features'
This is for all perf tests? That's bad because we would no longer be testing
existing behavior. Why not enable the flag within your test like layout tests
do?

[Justin Novosad, 2015-10-08 18:10:33]

On 2015/10/08 17:59:05, chrishtr wrote:
>
https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
> File tools/perf/benchmarks/blink_perf.py (right):
> 
>
https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
> tools/perf/benchmarks/blink_perf.py:90:
'--enable-experimental-canvas-features'
> This is for all perf tests? That's bad because we would no longer be testing
> existing behavior. Why not enable the flag within your test like layout tests
> do?

Not all perf, just blink_perf. 
Also, this flag simply unhides experimental APIs, it is not a behavior modifier.
for existing APIs. We run the layout tests with this flag always on (and a bunch
of other experimental API flags always on).

[chrishtr, 2015-10-08 18:13:11]

On 2015/10/08 at 18:10:33, junov wrote:
> On 2015/10/08 17:59:05, chrishtr wrote:
> >
https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
> > File tools/perf/benchmarks/blink_perf.py (right):
> > 
> >
https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
> > tools/perf/benchmarks/blink_perf.py:90:
'--enable-experimental-canvas-features'
> > This is for all perf tests? That's bad because we would no longer be testing
> > existing behavior. Why not enable the flag within your test like layout
tests
> > do?
> 
> Not all perf, just blink_perf. 
> Also, this flag simply unhides experimental APIs, it is not a behavior
modifier. for existing APIs. We run the layout tests with this flag always on
(and a bunch of other experimental API flags always on).

Maybe in this case it's ok then, since it should have no bearing on performance
of existing features.

[sullivan, 2015-10-09 18:41:01]

lgtm

blink_perf changes lgtm if we're not concerned about the affect on non-canvas
blink_perf tests.

[xidachen, 2015-10-13 15:09:25]

On 2015/10/08 17:42:20, Justin Novosad wrote:
> On 2015/10/08 17:31:38, xidachen wrote:
> > The "--extra-browser-args=--enable-experimental-canvas-features" should be
by
> > default for all blink_perf tests.
> > 
> > Run "tools/perf/run_benchmark --browser=default blink_perf_canvas" for this
> > particular test.
> 
> lgtm for third_party/WebKit
> Still need approval from sullivan@ for blink_perf.py

Thanks, Justin -:)

[xidachen, 2015-10-13 15:11:45]

On 2015/10/08 18:13:11, chrishtr wrote:
> On 2015/10/08 at 18:10:33, junov wrote:
> > On 2015/10/08 17:59:05, chrishtr wrote:
> > >
>
https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
> > > File tools/perf/benchmarks/blink_perf.py (right):
> > > 
> > >
>
https://codereview.chromium.org/1387933002/diff/40001/tools/perf/benchmarks/b...
> > > tools/perf/benchmarks/blink_perf.py:90:
> '--enable-experimental-canvas-features'
> > > This is for all perf tests? That's bad because we would no longer be
testing
> > > existing behavior. Why not enable the flag within your test like layout
> tests
> > > do?
> > 
> > Not all perf, just blink_perf. 
> > Also, this flag simply unhides experimental APIs, it is not a behavior
> modifier. for existing APIs. We run the layout tests with this flag always on
> (and a bunch of other experimental API flags always on).
> 
> Maybe in this case it's ok then, since it should have no bearing on
performance
> of existing features.

That's great, thank you, Chris.

[xidachen, 2015-10-13 15:17:00]

On 2015/10/09 18:41:01, sullivan wrote:
> lgtm
> 
> blink_perf changes lgtm if we're not concerned about the affect on non-canvas
> blink_perf tests.

Thank you for reviewing it -:).

[xidachen, 2015-10-13 15:17:24]

The CQ bit was checked by xidachen@chromium.org

[xidachen, 2015-10-13 15:17:26]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2015-10-13 15:19:44]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2015-10-13 15:19:49]

The CQ bit was checked by xidachen@chromium.org

[commit-bot: I haz the power, 2015-10-13 15:21:34]

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

[xidachen, 2015-10-13 15:25:33]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2015-10-13 16:57:16]

The CQ bit was checked by xidachen@chromium.org

[commit-bot: I haz the power, 2015-10-13 17:08:18]

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

[commit-bot: I haz the power, 2015-10-13 18:05:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-13 18:07:07]

Patchset 3 (id:??) landed as
https://crrev.com/9f0bb323d1b080be08797b13fa77e2d6153c5800
Cr-Commit-Position: refs/heads/master@{#353785}