Bug 98047 – Add a performance test for nested <use> elements

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

[Florin Malita]

Add a perf test to track progress on https://bugs.webkit.org/show_bug.cgi?id=97905.

[Florin Malita]

Created attachment 166504 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 166504 [details]
Patch

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

> PerformanceTests/SVG/SvgNestedUse.html:60
> +            times.push(Date.now());

You should use PerfTestRunner.now. It uses performance.webkitNow or Date.now, whichever is available.

> PerformanceTests/SVG/SvgNestedUse.html:62
> +            while (times.length > 10) { times.shift(); }

Nit: No curly brackets around a single statement. Also, times.shift() should be on the next line by itself indented by 4 spaces to the right of while.

> PerformanceTests/SVG/SvgNestedUse.html:63
> +            document.getElementById("FPS").textContent = (1000/fps).toFixed(2);

You don't want to just report fps instead?

> PerformanceTests/SVG/SvgNestedUse.html:65
> +            f++;

Please don't use one letter variable names.

> PerformanceTests/SVG/SvgNestedUse.html:79
> +        } else {
> +            requestAnimationFrame(animate);
> +        }

No curly brackets around a single statement.

[Florin Malita]

Created attachment 166510 [details]
Patch

[Florin Malita]

(In reply to comment #2)
> > PerformanceTests/SVG/SvgNestedUse.html:60
> > +            times.push(Date.now());
> 
> You should use PerfTestRunner.now. It uses performance.webkitNow or Date.now, whichever is available.

done.

> > PerformanceTests/SVG/SvgNestedUse.html:62
> > +            while (times.length > 10) { times.shift(); }
> 
> Nit: No curly brackets around a single statement. Also, times.shift() should be on the next line by itself indented by 4 spaces to the right of while.

done.

> > PerformanceTests/SVG/SvgNestedUse.html:63
> > +            document.getElementById("FPS").textContent = (1000/fps).toFixed(2);
> 
> You don't want to just report fps instead?

'fps' holds the average time for the last 10 frames at this point. Updated the name to reflect that.

> > PerformanceTests/SVG/SvgNestedUse.html:65
> > +            f++;
> 
> Please don't use one letter variable names.

done.
 
> > PerformanceTests/SVG/SvgNestedUse.html:79
> > +        } else {
> > +            requestAnimationFrame(animate);
> > +        }
> 
> No curly brackets around a single statement.

done.

[WebKit Review Bot]

Comment on attachment 166510 [details]
Patch

Clearing flags on attachment: 166510

Committed r130073: <http://trac.webkit.org/changeset/130073>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.