We need better test coverage in SVG to track performance changes.
Created attachment 148502 [details] Adds a performance test for paths in SVG
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?
Created attachment 148590 [details] Update per reviewer comments
(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.
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 :)
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.
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>
All reviewed patches have been landed. Closing bug.