Bug 89547

Summary: Add a performance test for paths in SVG
Product: WebKit Reporter: Philip Rogers <pdr>
Component: SVGAssignee: Philip Rogers <pdr>
Status: RESOLVED FIXED    
Severity: Normal CC: rniwa, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Adds a performance test for paths in SVG
none
Update per reviewer comments
rniwa: review+, rniwa: commit-queue-
Slow test down to ~70ms per run none

Philip Rogers
Reported 2012-06-19 21:45:50 PDT
We need better test coverage in SVG to track performance changes.
Attachments
Adds a performance test for paths in SVG (118.64 KB, patch)
2012-06-19 21:56 PDT, Philip Rogers
no flags
Update per reviewer comments (123.30 KB, patch)
2012-06-20 10:04 PDT, Philip Rogers
rniwa: review+
rniwa: commit-queue-
Slow test down to ~70ms per run (128.15 KB, patch)
2012-06-20 16:15 PDT, Philip Rogers
no flags
Philip Rogers
Comment 1 2012-06-19 21:56:59 PDT
Created attachment 148502 [details] Adds a performance test for paths in SVG
Ryosuke Niwa
Comment 2 2012-06-19 22:05:23 PDT
Comment on attachment 148502 [details] Adds a performance test for paths in SVG View in context: https://bugs.webkit.org/attachment.cgi?id=148502&action=review > PerformanceTests/ChangeLog:8 > + This change adds the first performance test for SVG paths. > + In the test we modify complex cubic paths in several ways, testing: > + transformations, clipping, d attribute changes, stroke properties, and opacity. Can we include sample outputs? > PerformanceTests/ChangeLog:10 > + Reviewed by NOBODY (OOPS!). This line should appear before the long description. > PerformanceTests/SVG/SvgCubics.html:858 > + }, 2, 100, function() { Do we really need 100 samples? How long does it take to run this test on, say, Mac mini?
Philip Rogers
Comment 3 2012-06-20 10:04:50 PDT
Created attachment 148590 [details] Update per reviewer comments
Philip Rogers
Comment 4 2012-06-20 10:09:18 PDT
(In reply to comment #2) > (From update of attachment 148502 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=148502&action=review > > > PerformanceTests/ChangeLog:8 > > + This change adds the first performance test for SVG paths. > > + In the test we modify complex cubic paths in several ways, testing: > > + transformations, clipping, d attribute changes, stroke properties, and opacity. > > Can we include sample outputs? Done (listed in the ChangeLog). For reference, the result will look something like: https://docs.google.com/file/d/0B4rN2u5hCJiUZmg0NTBob0Q0Ymc/edit?pli=1 > > > PerformanceTests/ChangeLog:10 > > + Reviewed by NOBODY (OOPS!). > > This line should appear before the long description. Done. > > > PerformanceTests/SVG/SvgCubics.html:858 > > + }, 2, 100, function() { > > Do we really need 100 samples? How long does it take to run this test on, say, Mac mini? 100 was not necessary as the sample output using just 20 shows that the std dev is quite reasonable. On my linux desktop this test now takes less than 1 second to run, and 2 seconds total including starting up the test script.
Ryosuke Niwa
Comment 5 2012-06-20 12:14:10 PDT
Comment on attachment 148590 [details] Update per reviewer comments View in context: https://bugs.webkit.org/attachment.cgi?id=148590&action=review > PerformanceTests/ChangeLog:14 > + median= 32.0 ms, stdev= 2.92403830344 ms, min= 22.0 ms, max= 37.0 ms We probably want to tweak the test so that's median is well over 50ms or even 100ms in the case things get faster in the future :)
Philip Rogers
Comment 6 2012-06-20 16:15:26 PDT
Created attachment 148672 [details] Slow test down to ~70ms per run Add even more path data to slow the test down to ~70ms per run which should get us out of the range of accuracy of JavaScript timers.
WebKit Review Bot
Comment 7 2012-06-20 18:04:18 PDT
Comment on attachment 148672 [details] Slow test down to ~70ms per run Clearing flags on attachment: 148672 Committed r120892: <http://trac.webkit.org/changeset/120892>
WebKit Review Bot
Comment 8 2012-06-20 18:04:25 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.