Bug 98047 - Add a performance test for nested <use> elements
Summary: Add a performance test for nested <use> elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Florin Malita
URL:
Keywords:
Depends on:
Blocks: 77037 97905
  Show dependency tree / graph
 
Reported: 2012-10-01 10:39 PDT by Florin Malita
Modified: 2012-10-01 13:53 PDT (History)
6 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Florin Malita 2012-10-01 10:39:28 PDT 
Add a perf test to track progress on https://bugs.webkit.org/show_bug.cgi?id=97905.
Comment 1 Florin Malita 2012-10-01 10:49:27 PDT 
Created attachment 166504 [details]
Patch
Comment 2 Ryosuke Niwa 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.
Comment 3 Florin Malita 2012-10-01 11:30:36 PDT 
Created attachment 166510 [details]
Patch
Comment 4 Florin Malita 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.
Comment 5 WebKit Review Bot 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>
Comment 6 WebKit Review Bot 2012-10-01 13:53:36 PDT 
All reviewed patches have been landed.  Closing bug.