[webkit-reviews] review denied: [Bug 105513] Add a performance test for Accelerated Compositing. : [Attachment 180313] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Hanyee Kim
<choco at company100.net>'s request for review:
Bug 105513: Add a performance test for Accelerated Compositing.
https://bugs.webkit.org/show_bug.cgi?id=105513

Attachment 180313: Patch
https://bugs.webkit.org/attachment.cgi?id=180313&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=180313&action=review


> PerformanceTests/Animation/resources/leaves.js:178
> +window.onload = function () {
> +    PerfTestRunner.prepareToMeasureValuesAsync({iterationCount: 10, done:
onCompletedRun, unit: 'fps'});
> +    window.webkitRequestAnimationFrame(onAnimationFrame);
> +}

window.addEventListener('load', ...) is better style.

> PerformanceTests/Animation/resources/leaves.js:201
> +function onAnimationFrame() {
> +    if ((iteration++ % FRAMES_PER_TIMER_READING) == 0) {
> +	   var currTime = new Date();
> +
> +	   // Calculate the framerate based upon the difference between the
absolute times of the oldest and newest frames, subdivided by how macany frames
were drawn inbetween
> +	   var frameRateVal = (FRAMES_PER_TIMER_READING * 1000) / (currTime -
lastMeasuredTime);
> +	   if (!isNaN(frameRateVal))
> +	       PerfTestRunner.measureValueAsync(frameRateVal);
> +
> +	   lastMeasuredTime = currTime;
> +    }
> +    if (!measureCompleted) {
> +	   window.webkitRequestAnimationFrame(onAnimationFrame);
> +    }
> +}

You're assuming that RAF frequency is related to the frequency with which CSS
animations are rendered. This is not true on some platforms, so I don't think
this is really a useful way to measure CSS animation performance.

There are other interesting things to measure: how fast this animations runs
when driven via RAF, or how expensive it is to do layout on a page with lots of
accelerated compositing layers. But I don't think this test useful for all
platforms.