Bug 78789 – Import themaninblue.com/experiment/AnimationBenchmark/ as performance tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Ryosuke Niwa]

Per my request, the author of http://themaninblue.com/experiment/AnimationBenchmark/html/ have LGPL-licensed his animation benchmarks: http://themaninblue.com/writing/perspective/2010/03/22/
We should import them as performance tests.

[Tom Zakrajsek]

Created attachment 145649 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 145649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145649&action=review

> Tools/Scripts/webkitpy/performance_tests/perftest.py:95
> +        re.compile(r'^\d+(.\d+)?(\s*(runs\/s|ms|FPS|fps))?$'),

Why do we need to support both FPS and fps? To be consistent, I'd prefer just supporting the lowercase fps.

> PerformanceTests/Animation/balls.html:272
> +        // Get the balls and FPS out of our way

I don't think this comment is helpful.

> PerformanceTests/Animation/balls.html:278
> +        for (var particle in particles)
> +        {
> +            var p = particles[particle];
> +            particles[particle] = 0;
> +            p.destroy();
> +        }

Please use WebKit style for { and } (e.g. for (...) {).

> PerformanceTests/Animation/balls.html:289
> +        for (var rate in frameRates)
> +        {
> +            PerfTestRunner.log(frameRates[rate].toFixed(2) + " FPS");
> +        }

No curly brackets around a single line statement.

> PerformanceTests/Animation/balls.html:305
> +        if (++run < maxRuns)
> +        {
> +            init();
> +        }
> +        else
> +        {
> +            if (window.layoutTestController) {
> +                layoutTestController.notifyDone();
> +            }
> +        }

Ditto.

[Tom Zakrajsek]

Thanks for the feedback.  I thought minimizing change was more important than fitting our style. I'll fix it.  And, i'll restrict to fps.  I also realized that the stdev goes down *significantly* if I widen the resolution of the timing to every ten screen updates.  Average and Median frame rates stay about the same.  Any moral problems with that?

[Ryosuke Niwa]

(In reply to comment #3)
> Thanks for the feedback.  I thought minimizing change was more important than fitting our style. I'll fix it.  And, i'll restrict to fps.  I also realized that the stdev goes down *significantly* if I widen the resolution of the timing to every ten screen updates.  Average and Median frame rates stay about the same.  Any moral problems with that?

Whatever works for you. Could you also report post the sample results (4-5 runs) somewhere and tell us how long it takes to run the test?

[Tom Zakrajsek]

Created attachment 148161 [details]
Patch

[Tom Zakrajsek]

[tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
Running 1 tests
Running Animation/balls.html (1 of 1)
RESULT Animation: balls= 169.223297408 fps
median= 168.918918919 fps, stdev= 7.89349356211 fps, min= 160.0 fps, max= 192.307692308 fps
real 2.749	user 2.464	sys 0.485	pcpu 100.00

[tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
Running 1 tests
Running Animation/balls.html (1 of 1)
RESULT Animation: balls= 167.750742674 fps
median= 166.270783848 fps, stdev= 8.07382328862 fps, min= 160.0 fps, max= 192.307692308 fps
real 2.759	user 2.485	sys 0.486	pcpu 100.00

[tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
Running 1 tests
Running Animation/balls.html (1 of 1)
RESULT Animation: balls= 168.481646673 fps
median= 167.714884696 fps, stdev= 8.06403523694 fps, min= 158.73015873 fps, max= 192.307692308 fps
real 2.739	user 2.469	sys 0.486	pcpu 100.00

[tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
Running 1 tests
Running Animation/balls.html (1 of 1)
RESULT Animation: balls= 165.902753749 fps
median= 164.705882353 fps, stdev= 7.51791081831 fps, min= 158.73015873 fps, max= 188.679245283 fps
real 2.760	user 2.476	sys 0.489	pcpu 100.00

[tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
Running 1 tests
Running Animation/balls.html (1 of 1)
RESULT Animation: balls= 171.81506867 fps
median= 171.30620985 fps, stdev= 8.34146408445 fps, min= 161.290322581 fps, max= 196.078431373 fps
real 2.728	user 2.455	sys 0.487	pcpu 100.00

[Ryosuke Niwa]

Comment on attachment 148161 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148161&action=review

> PerformanceTests/Animation/balls.html:263
> +        else
> +            if (window.layoutTestController)
> +                layoutTestController.notifyDone();

Please put if & else on the same line.

[Tim Horton]

(In reply to comment #6)
> [tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
> RESULT Animation: balls= 169.223297408 fps
> median= 168.918918919 fps, stdev= 7.89349356211 fps, min= 160.0 fps, max= 192.307692308 fps

What's the WebKit performance test policy on importing tests that are ridiculously fast? 160fps is crazy, and some systems might have optimizations that perhaps make the testing unfair. Do we have a "target" fps, or a mechanism for scaling tests, or anything?

[Ryosuke Niwa]

(In reply to comment #8)
> (In reply to comment #6)
> > [tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
> > RESULT Animation: balls= 169.223297408 fps
> > median= 168.918918919 fps, stdev= 7.89349356211 fps, min= 160.0 fps, max= 192.307692308 fps
> 
> What's the WebKit performance test policy on importing tests that are ridiculously fast? 160fps is crazy, and some systems might have optimizations that perhaps make the testing unfair. Do we have a "target" fps, or a mechanism for scaling tests, or anything?

We don't have a policy :) If we think that's too fast, maybe we can tweak it to add more load?

[Simon Fraser (smfr)]

Any test that runs > 60fps is useless as a perf test. It's just testing throttling at various places in the whole stack.

[Tim Horton]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > [tomz-mac-pro] WebKit> time run-perf-tests PerformanceTests/Animation/balls.html
> > > RESULT Animation: balls= 169.223297408 fps
> > > median= 168.918918919 fps, stdev= 7.89349356211 fps, min= 160.0 fps, max= 192.307692308 fps
> > 
> > What's the WebKit performance test policy on importing tests that are ridiculously fast? 160fps is crazy, and some systems might have optimizations that perhaps make the testing unfair. Do we have a "target" fps, or a mechanism for scaling tests, or anything?
> 
> We don't have a policy :) If we think that's too fast, maybe we can tweak it to add more load?

I use this test (and its siblings) with MAX_PARTICLES set to 10,000 :P

This (having a target speed, etc.) is probably something we should look into sooner rather than later; as graphics subsystems get smarter, there's no reason they can't throw out or defer work that won't make it to the screen immediately (or ever), and there's potential for system-level fps capping that we should watch out for.

In any case, something to think about.

[Tom Zakrajsek]

One note about the high FPS... this actually runs slower (like 1/3 -1/2 the speed) when you run it in a browser.  But scaling the FPS is trivial.  If you triple MAX_PARTICLES, the rate goes down proportionally.  I'll update with the nit fix.

[Ryosuke Niwa]

So... should we land this patch? Or should we tweak it first?

[Tom Zakrajsek]

Created attachment 148558 [details]
Patch

[Tom Zakrajsek]

Comment on attachment 148558 [details]
Patch

Clearing flags on attachment: 148558

Committed r120823: <http://trac.webkit.org/changeset/120823>

[Tom Zakrajsek]

All reviewed patches have been landed.  Closing bug.

[Dominik Röttsches (drott)]

Comment on attachment 148558 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148558&action=review

> Tools/Scripts/webkitpy/performance_tests/perftest.py:138
> +                _log.error("PerfTest can't parse line [" + line + "]")

Looks like this causes a python tests regression:
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/1653/steps/webkitpy-test/logs/stdio