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: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 77037, 97905 | ||||||||
Attachments: |
|
Description
Florin Malita
2012-10-01 10:39:28 PDT
Created attachment 166504 [details]
Patch
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. Created attachment 166510 [details]
Patch
(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. Comment on attachment 166510 [details] Patch Clearing flags on attachment: 166510 Committed r130073: <http://trac.webkit.org/changeset/130073> All reviewed patches have been landed. Closing bug. |