Bug 98047

Summary:

Add a performance test for nested <use> elements

Product:

WebKit

Reporter:

Florin Malita <fmalita>

Component:

Tools / Tests

Assignee:

Florin Malita <fmalita>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

krit, pdr, rniwa, schenney, webkit.review.bot, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

77037, 97905

Attachments:

Description	Flags
Patch	none
Patch	none

Florin Malita

Reported 2012-10-01 10:39:28 PDT

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

Attachments
Patch (4.28 KB, patch) 2012-10-01 10:49 PDT, Florin Malita	no flags	Details Formatted Diff Diff
Patch (4.30 KB, patch) 2012-10-01 11:30 PDT, Florin Malita	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Florin Malita

Comment 1 2012-10-01 10:49:27 PDT

Created attachment 166504 [details] Patch

Ryosuke Niwa

Comment 2 2012-10-01 11:06:08 PDT

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

Comment 3 2012-10-01 11:30:36 PDT

Created attachment 166510 [details] Patch

Florin Malita

Comment 4 2012-10-01 11:33:19 PDT

(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 5 2012-10-01 13:53:33 PDT

Comment on attachment 166510 [details] Patch Clearing flags on attachment: 166510 Committed r130073: <http://trac.webkit.org/changeset/130073>

WebKit Review Bot

Comment 6 2012-10-01 13:53:36 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.