Bug 30055 – Bad DOM performance in large SVG files

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Patrick R. Gansterer]

When you click on the 1st rect all rects get yellow, on the 2nd they get blue.
When you click on the 3rd the first rect gets lime, on the 4th it gets red.

Setting only one rect is as slow as setting all rects.
In Firefox 3 you see a big difference between setting all or only one rect.

With the c-parameter you can change the count of the rects, with the s-parameter the size.

Setting only one rect is much faster in http://paroga.com/webkit/rect.php?c=4 than in http://paroga.com/webkit/rect.php?c=10000.
In Firefox there is no difference.

[Patrick R. Gansterer]

Most of the time ist spent in:
RenderLayer::paintLayer
RenderSVGRoot::paint

It seems that the complete SVG will be rendered every time.

I tested it with the Qt port where the dirtyRect is always the size of the whole frame.

[Patrick R. Gansterer]

The same page as html (HTML:div insted of SVG:rect):
http://paroga.com/webkit/div.php?c=4
http://paroga.com/webkit/div.php?c=10000

When only setting a single rect (click on rect 3 or 4) the HTML version is much faster. When changing the color of all rects the SVG version is faster.

[Eric Seidel (no email)]

I'm not sure I see the difference.  WebKit only seems to be drawing the SVG rects as their added.  Using Quartz Debug, I do not see it repainting the whole SVG every time.

Bug 14015 is sorta related (drawing too much).

I think we need to see a better example to see the performance difference, if there is one.  Your example does not convince me that we're drawing too much.  However having written the code, I believe you that we're drawing too much. :)

[Patrick R. Gansterer]

(In reply to comment #3)
> I'm not sure I see the difference.  WebKit only seems to be drawing the SVG
> rects as their added.  Using Quartz Debug, I do not see it repainting the whole
> SVG every time.
Have you tried it wit rect.php?c=10000&s=3? Maybay it there to many rects out of the frame, you can't see a difference. It is not the inital rendering, you can see the difference only when click on rect 1, 2, 3 or 4.

I think that the dirtyRegion is calculatet correctly, but nothing cares about it (at least in the Qt port)

[Eric Seidel (no email)]

OK, so clicking is important, yes.  I didn't realize I had to click on them.

I tried these just now in Safari 4.0.3 and SVG was correctly redrawing only one svg rect when toggling the color on one rect.  If Qt is re-drawing the whole page, that seems like a qt bug.  I did not try on tip of tree, but I kinda doubt this regressed... but maybe it did.

[Patrick R. Gansterer]

(In reply to comment #5)
1) open http://paroga.com/webkit/rect.php?c=4&s=3
2) click on the 1st rect and stop the time until the first rect changes its color
3) click on the 3rd rect and stop the time until the first rect changes its color
4) open http://paroga.com/webkit/rect.php?c=10000&s=3
5) click on the 1st rect and stop the time until the first rect changes its color
6) click on the 3rd rect and stop the time until the first rect changes its color
7) time in step 6 is much bigger than in step 3

Both times should be the same, because only one rect changes it's color. When time in step 2 is much bigger than in step 5 this is ok, because in the second page are much more items to be redrawn.

You can see the expected result when you open the page with Firefox.

[Patrick R. Gansterer]

Created attachment 41379 [details]
Paint RenderPath only when necessary

With this path clicking on the 3rd or 4th rect in http://paroga.com/webkit/rect.php?c=10000&s=3 is about 4 times faster on my computer. (time from click event until the first rect changes it's color)
I'm not sure if this is the right place for my changes.

[Eric Seidel (no email)]

This looks great!  I'd like to test the patch myself first before I r+ this though.

[Oliver Hunt]

Comment on attachment 41379 [details]
Paint RenderPath only when necessary

Why are you checking for intersection prior to applying the local transform to the paintInfo region?

[Patrick R. Gansterer]

(In reply to comment #9)
> Why are you checking for intersection prior to applying the local transform to
> the paintInfo region?
I only tried to find an easy way to demonstate the performance problem. The paintInfo.context->save(); stuff takes a significant amount of time on my machine. In my optinion there must be a better way to handle "DOM repaints" in general. Always iterateing over _all_ items isn't very cool.
Maybe you can confirm this issue and tell me the prefered solution.

If you take a look at http://paroga.com/webkit/livedemo.php you can see a (not so good) example what people do with SVG.

[Eric Seidel (no email)]

This patch is exactly the right idea.  I just need to confirm that it's implemented correctly.  I need to make space in my schedule to test your patch locally and check to see if other if statements like this one should be added to SVG renderers.

[Eric Seidel (no email)]

Ok, I'm not able to reproduce the slowness you mention about clicking on the 3rd or 4th rect in http://paroga.com/webkit/rect.php?c=10000&s=3.

That example is actually really poor, because it requires someone to know that they're supposed to click on a special rect to demonstrate the bug.  Much better would be a self-documenting page with buttons one could click on which say what they do.

I agree we should probably make a change like this.  I just don't think we've justified it very well up until now.

The pathalogical case that you're trying to demonstrate is a single group containing a large number of path objects (<rect>, etc) where only one of them changes.  In current SVG code, all of them will be asked to paint and then the results will be clipped out. It appears that the clipping is rather cheap in CoreGraphics, but whatever platform you are using is slower.

[Patrick R. Gansterer]

(In reply to comment #12)
> That example is actually really poor, because it requires someone to know that
> they're supposed to click on a special rect to demonstrate the bug.  Much
> better would be a self-documenting page with buttons one could click on which
> say what they do.
I've improved the example.

[Patrick R. Gansterer]

(In reply to comment #12)
> Ok, I'm not able to reproduce the slowness you mention about clicking on the
> 3rd or 4th rect in http://paroga.com/webkit/rect.php?c=10000&s=3.
I can't believe you! ;-) I now compiled r50334 with the Qt-port (4.6 beta) on windows and tested the following patch (with g_checkIntersect true and false):
>  if (paintInfo.context->paintingDisabled() || style()->visibility() ...)
>          return;
> 
> +    FloatRect boundingBox = repaintRectInLocalCoordinates();
> +    if (g_checkIntersect && !boundingBox.intersects(paintInfo.rect))
> +        return;
> +
>      paintInfo.context->save();
>      paintInfo.context->concatCTM(localToParentTransform());
>  
>      SVGResourceFilter* filter = 0;
>  
> -    FloatRect boundingBox = repaintRectInLocalCoordinates();
>      if (paintInfo.phase == PaintPhaseForeground) {
>          PaintInfo savedInfo(paintInfo);
When I open http://paroga.com/webkit/rect.php?c=80000&s=3 with g_checkIntersect = true it takes about 2 seconds to change the color of the first element. With g_checkIntersect = false the same action takes about 10 seconds on my machine.

Maybe the rendering/clipping isn't as time-consuming with your port? Is it possible that you try it with the Qt-port? When i apply this patch to WebCore it can't be a Qt-specific rendering problem?!?

[Oliver Hunt]

> Maybe the rendering/clipping isn't as time-consuming with your port? Is it
> possible that you try it with the Qt-port? When i apply this patch to WebCore
> it can't be a Qt-specific rendering problem?!?

WebCore uses the platform's graphics library, if that graphics library has bad perf in some cases it can easily cause all sorts of badness.  I'm unsure whether the solution should be to get Qt to fix this issue (presumably it's a problem in their graphics library) or fix it in WebCore itself (which may negatively impact performance of other platforms)

Maybe this logic should be in GrpahicsContextQt?

[Patrick R. Gansterer]

(In reply to comment #15)
> WebCore uses the platform's graphics library, if that graphics library has bad
> perf in some cases it can easily cause all sorts of badness.  I'm unsure
> whether the solution should be to get Qt to fix this issue (presumably it's a
> problem in their graphics library) or fix it in WebCore itself (which may
> negatively impact performance of other platforms)
> 
> Maybe this logic should be in GrpahicsContextQt?
Do you think rendering anything ist better than checking if it is necessary? I think WebCore should render as least as possible. In GrpahicsContextQt there won't be any information to check if painting is required. Rely on a "perfect performing" graphics library isn't the right way in opinion. There might be some other ports with slow graphics librarys too.

[Oliver Hunt]

(In reply to comment #16)
> (In reply to comment #15)
> > WebCore uses the platform's graphics library, if that graphics library has bad
> > perf in some cases it can easily cause all sorts of badness.  I'm unsure
> > whether the solution should be to get Qt to fix this issue (presumably it's a
> > problem in their graphics library) or fix it in WebCore itself (which may
> > negatively impact performance of other platforms)
> > 
> > Maybe this logic should be in GrpahicsContextQt?
> Do you think rendering anything ist better than checking if it is necessary? I
> think WebCore should render as least as possible. In GrpahicsContextQt there
> won't be any information to check if painting is required. Rely on a "perfect
> performing" graphics library isn't the right way in opinion. There might be
> some other ports with slow graphics librarys too.

The problem is that if the underlying graphics library is already doing the tests it may have a more efficient way of doing so, add to that the fact that the underlying graphics libraries that do perform sensible clipping optimisations will still be doing their own clip tests this seems like a patch that has bad perf impact on good libraries to deal with poor performance in others.

I suspect if we really do want webcore to work around what is arguably a perf bug in Qts graphics subsystem the fix should be placed in GraphicsContextQt directly

[Eric Seidel (no email)]

I think we can use both approaches, and I think that this approach has some usefulness.  It needs to be well tested though.  I also agree with Oliver, that if Qt is not properly ignoring drawing commands which are issued outside of the clip region, than that seems like a major bug in Qt.

[Patrick R. Gansterer]

(In reply to comment #17)
> The problem is that if the underlying graphics library is already doing the
> tests it may have a more efficient way of doing so, add to that the fact that
> the underlying graphics libraries that do perform sensible clipping
> optimisations will still be doing their own clip tests this seems like a patch
> that has bad perf impact on good libraries to deal with poor performance in
> others.
So this patch has to move to a higher level where "more intelligent" test can
be done and the clipping should be fixed in GraphicsContextQt directly.

However the following test is much slower(!) in Safari/Chrome than in FF:
Click on "Set first lime/red" at http://paroga.com/webkit/rect.php?c=80000&s=2
On my machine: FF ~150ms, Opera ~300ms, Chrome ~2s, Safari ~10s

How to get the same performance of FF/Opera in WebCore? Any suggestions?

[Adam Barth]

Attachment 41379 [details] passed the style-queue

[Oliver Hunt]

(In reply to comment #19)
> (In reply to comment #17)
> > The problem is that if the underlying graphics library is already doing the
> > tests it may have a more efficient way of doing so, add to that the fact that
> > the underlying graphics libraries that do perform sensible clipping
> > optimisations will still be doing their own clip tests this seems like a patch
> > that has bad perf impact on good libraries to deal with poor performance in
> > others.
> So this patch has to move to a higher level where "more intelligent" test can
> be done and the clipping should be fixed in GraphicsContextQt directly.
> 
> However the following test is much slower(!) in Safari/Chrome than in FF:
> Click on "Set first lime/red" at http://paroga.com/webkit/rect.php?c=80000&s=2
> On my machine: FF ~150ms, Opera ~300ms, Chrome ~2s, Safari ~10s
> 
> How to get the same performance of FF/Opera in WebCore? Any suggestions?

I don't see this 10s delay you're referring to what platform are you testing on?

[Patrick R. Gansterer]

(In reply to comment #21)
> I don't see this 10s delay you're referring to what platform are you testing
> on?
Safari 4.0.4, Windows XP SP3, IBM T60 2GHz, 3GB

[Oliver Hunt]

Comment on attachment 41379 [details]
Paint RenderPath only when necessary

I found another testcase where we get pathologically bad perf due to this issue which would imply that there's just too much cost in not clipping prior to calling the graphics library -- my test case just created many many many (~50000 odd) nodes mostly off the screen and this patch represents a huge improvement in that case.

Still not sure why i don't see the perf problems you do in your example, but with a testcase that is too slow this shows up as a clear problem in shark.

r=me.

The patch applies cleanly lets see how the commit bot deals with the changelog.

[WebKit Commit Bot]

Comment on attachment 41379 [details]
Paint RenderPath only when necessary

Rejecting patch 41379 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11771 test cases.
svg/batik/filters/filterRegions.svg -> failed

Exiting early after 1 failures. 10019 tests run.
313.92s total testing time

10018 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
6 test cases (<1%) had stderr output

[Dirk Schulze]

(In reply to comment #23)
> (From update of attachment 41379 [details])
> I found another testcase where we get pathologically bad perf due to this issue
> which would imply that there's just too much cost in not clipping prior to
> calling the graphics library -- my test case just created many many many
> (~50000 odd) nodes mostly off the screen and this patch represents a huge
> improvement in that case.

offscreen:
It depends. If the SVG size is also bigger than the screensize, we shouldn't clip to the currently visible area. smfr remind me, that we can include a SVG into a HTML-file and make some 3D transformations (via CSS3) with it. That might not work correctly, if the SVG is clipped.

[Oliver Hunt]

(In reply to comment #25)
> (In reply to comment #23)
> > (From update of attachment 41379 [details] [details])
> > I found another testcase where we get pathologically bad perf due to this issue
> > which would imply that there's just too much cost in not clipping prior to
> > calling the graphics library -- my test case just created many many many
> > (~50000 odd) nodes mostly off the screen and this patch represents a huge
> > improvement in that case.
> 
> offscreen:
> It depends. If the SVG size is also bigger than the screensize, we shouldn't
> clip to the currently visible area. smfr remind me, that we can include a SVG
> into a HTML-file and make some 3D transformations (via CSS3) with it. That
> might not work correctly, if the SVG is clipped.

We should simply be clipping to the region that is going to actually be drawn to, assuming everything is working correctly that is correct -- if i can apply a 3d transform and that will result in incorrect clipping area then thats a serious bug in the 3d transform paths

[Eric Seidel (no email)]

Curious what the status of this patch is?  Should this be r-'d now that we know that this patch will at least cause one layout test to fail?

[Eric Seidel (no email)]

Comment on attachment 41379 [details]
Paint RenderPath only when necessary

Marking r- since this causes a test to fail.

[Eric Seidel (no email)]

Also the second OOPS in the ChangeLog would have caused the commit to fail.  ChangeLogs should mention tests affected by a change.

[Patrick R. Gansterer]

(In reply to comment #28)
> (From update of attachment 41379 [details])
> Marking r- since this causes a test to fail.
The problem is that an other area of Webkit does something wrong. This patch is ok, but i depends on an "not created bug". Can we create a new bug, make the patch depend on in r+ it again?  Who is responsible for the code tested with filterRegions.svg? Maybe he can make a change there to get this patch in?

[Eric Seidel (no email)]

Yes, if we're waiting to commit this because a problem exists that we've never actually created a bug for, then definitely we should create a bug. :)

Getting an r+ is relatively easy.  Especially since this patch already had one.  But if we can't land it due to another bug, we should file the other bug and mark this one blocked.

[Dirk Schulze]

Can we move this fix to SVGRenderBase::prepareToRenderSVGContent??

This would stop rendering earlier and can improve the rendering time on other parts like images too.

[Patrick R. Gansterer]

(In reply to comment #31)
> Yes, if we're waiting to commit this because a problem exists that we've never
> actually created a bug for, then definitely we should create a bug. :)
Can we r+ and cq+ it again since bug 32815 is fixed now?

(In reply to comment #32)
> Can we move this fix to SVGRenderBase::prepareToRenderSVGContent??
> 
> This would stop rendering earlier and can improve the rendering time on other
> parts like images too.
I think we should commit this change and maybe another too. This patch is not in the SVG part so it might improve also some other parts.

[Dirk Schulze]

> (In reply to comment #32)
> (In reply to comment #32)
> > Can we move this fix to SVGRenderBase::prepareToRenderSVGContent??
> > 
> > This would stop rendering earlier and can improve the rendering time on other
> > parts like images too.
> I think we should commit this change and maybe another too. This patch is not
> in the SVG part so it might improve also some other parts.

RenderPath is SVG only.

[Patrick R. Gansterer]

(In reply to comment #34)
> RenderPath is SVG only.
I meant that maybe another componet will use RenderPath in the future, but thats not a real reason.

Moving it to prepareToRenderSVGContent won't prevent _unnecessary_ paintInfo.context->save(), concatCTM(...), restore() and so on.
It should be done as early as possible! Iterating over _all_ items, even if only one pixel changed, is already not very performant. (checking which items need a repaint might be even more cpu consuming)

[Dirk Schulze]

(In reply to comment #35)
> (In reply to comment #34)
> > RenderPath is SVG only.
> I meant that maybe another componet will use RenderPath in the future, but
> thats not a real reason.
> 
> Moving it to prepareToRenderSVGContent won't prevent _unnecessary_
> paintInfo.context->save(), concatCTM(...), restore() and so on.
> It should be done as early as possible! Iterating over _all_ items, even if
> only one pixel changed, is already not very performant. (checking which items
> need a repaint might be even more cpu consuming)

Can you please upload a new patch with a link to the bugreport Changelog and the title of the bug? There shouldn't be Oops'es in a Changelog. Please write why it is not possible to test the speed improvement and I think we can commit it.

[Patrick R. Gansterer]

Created attachment 45658 [details]
Paint RenderPath only when necessary

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45658 [details] without any errors.

[Dirk Schulze]

Comment on attachment 45658 [details]
Paint RenderPath only when necessary

LGTM. Thanks for the patch.

[WebKit Commit Bot]

Comment on attachment 45658 [details]
Paint RenderPath only when necessary

Rejecting patch 45658 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueueSVN/LayoutTests
Testing 11854 test cases.
svg/custom/marker-overflow-clip.svg -> failed

Exiting early after 1 failures. 10306 tests run.
313.90s total testing time

10305 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
7 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/152351

[Dirk Schulze]

(In reply to comment #40)
> (From update of attachment 45658 [details])
> Rejecting patch 45658 from commit-queue.
> 
> Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari',
> '--exit-after-n-failures=1', '--quiet']" exit_code: 1
> Running build-dumprendertree
> Running tests from /Users/eseidel/Projects/CommitQueueSVN/LayoutTests
> Testing 11854 test cases.
> svg/custom/marker-overflow-clip.svg -> failed
> 
> Exiting early after 1 failures. 10306 tests run.
> 313.90s total testing time
> 
> 10305 test cases (99%) succeeded
> 1 test case (<1%) had incorrect layout
> 7 test cases (<1%) had stderr output
> 
> Full output: http://webkit-commit-queue.appspot.com/results/152351

Ahhh sorry. There is another bug about markers and boundingBoxes: bug 33012 . Without knowing the actual result with your patch, I guess that this fixes the problem.

[Eric Seidel (no email)]

Comment on attachment 45658 [details]
Paint RenderPath only when necessary

Marking r- since this seems to cause a test to fail?  Correct me if I'm wrong Dirk.

[Dirk Schulze]

(In reply to comment #42)
> (From update of attachment 45658 [details])
> Marking r- since this seems to cause a test to fail?  Correct me if I'm wrong
> Dirk.
No, the patch is ok. It just depends on anotherone. I'll commot-queue+ the patch again, once Niko fixed the marker bug.

[Dirk Schulze]

Comment on attachment 45658 [details]
Paint RenderPath only when necessary

Markers and repaintRects are fixed. Lets try it again.

[WebKit Commit Bot]

Comment on attachment 45658 [details]
Paint RenderPath only when necessary

Clearing flags on attachment: 45658

Committed r52900: <http://trac.webkit.org/changeset/52900>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Comment on attachment 45658 [details]
Paint RenderPath only when necessary

This type of fix would be more interesting to apply to the hit-testing code path.  But this fix should be fine.  Thanks for the patch Patrick and the review Dirk!

[Dimitri Glazkov (Google)]

This change broke a bunch of SVG tests, all pixel results. For illustration, visit this at ToT: http://trac.webkit.org/export/52929/trunk/LayoutTests/fast/backgrounds/size/contain-and-cover.html

I think we should roll this change out.

[Dimitri Glazkov (Google)]

Reverted as http://trac.webkit.org/changeset/52930.

[Dirk Schulze]

(In reply to comment #48)
> This change broke a bunch of SVG tests, all pixel results. For illustration,
> visit this at ToT:
> http://trac.webkit.org/export/52929/trunk/LayoutTests/fast/backgrounds/size/contain-and-cover.html
> 
> I think we should roll this change out.

A problem of transparent and half transparent parts of some objects?

[Oliver Hunt]

Created attachment 46243 [details]
Bad DOM performance in large SVG files

[Oliver Hunt]

Created attachment 46244 [details]
Bad DOM performance in large SVG files

[Nikolas Zimmermann]

Comment on attachment 46243 [details]
Bad DOM performance in large SVG files

LGTM, I assume there are no pixel test regressions, r+.

[Nikolas Zimmermann]

Comment on attachment 46244 [details]
Bad DOM performance in large SVG files

Interessting that CGPathIsEmpty is that slow... I assume you see a good benefit, so r=me.

[Oliver Hunt]

First patch landed in r53057.

I'm actually slightly uneasy about landing the isempty caching so i'm going to see if there's something that can be done to get a bigger win.  That said i think that this patch should resolve all of the perf issues in the main testcase attached to this patch.

[Dimitri Glazkov (Google)]

Sorry guys, still no luck on pixel tests. Running "run-webkit-tests --pixel-tests svg" produces: 

Enabling pixel tests with a tolerance of 0.1%
WARNING: Temporarily changing the main display color profile:
	The colors on your screen will change for the duration of the testing.
	This allows the pixel tests to have consistent color values across all machines.
Testing 945 test cases.
svg/W3C-SVG-1.1 .......................................................................
svg/W3C-SVG-1.1/coords-units-01-b.svg -> pixel test failed
.........
svg/W3C-SVG-1.1/filters-composite-02-b.svg -> pixel test failed
..........................................
svg/W3C-SVG-1.1/masking-intro-01-f.svg -> pixel test failed
.
svg/W3C-SVG-1.1/masking-mask-01-b.svg -> pixel test failed
......................................
svg/W3C-SVG-1.1/pservers-grad-03-b.svg -> pixel test failed
...
svg/W3C-SVG-1.1/pservers-grad-06-b.svg -> pixel test failed
.......
svg/W3C-SVG-1.1/pservers-grad-13-b.svg -> pixel hash failed (but pixel test still passes)
....
svg/W3C-SVG-1.1/pservers-grad-17-b.svg -> pixel hash failed (but pixel test still passes)
...
svg/W3C-SVG-1.1/pservers-pattern-01-b.svg -> pixel test failed
.......................................................................
svg/W3C-SVG-1.1/text-deco-01-b.svg -> pixel hash failed (but pixel test still passes)
....
svg/W3C-SVG-1.1/text-path-01-b.svg -> pixel hash failed (but pixel test still passes)
...
svg/W3C-SVG-1.1/text-text-03-b.svg -> pixel hash failed (but pixel test still passes)
....
svg/W3C-SVG-1.1/text-text-07-t.svg -> pixel hash failed (but pixel test still passes)
........
svg/animations .
svg/batik/filters ..
svg/batik/masking .
svg/batik/masking/maskRegions.svg -> pixel test failed
svg/batik/paints ..
svg/batik/paints/patternPreserveAspectRatioA.svg -> pixel test failed
.
svg/batik/paints/patternRegionA.svg -> pixel test failed
.
svg/batik/paints/patternRegions.svg -> pixel test failed
svg/batik/text .....
svg/batik/text/textDecoration2.svg -> pixel hash failed (but pixel test still passes)
..
svg/batik/text/textEffect3.svg -> pixel hash failed (but pixel test still passes)
..
svg/batik/text/textFeatures.svg -> pixel hash failed (but pixel test still passes)
.
svg/batik/text/textGlyphOrientationHorizontal.svg -> pixel hash failed (but pixel test still passes)
.......
svg/batik/text/textOnPathSpaces.svg -> pixel hash failed (but pixel test still passes)
.....
svg/batik/text/textProperties.svg -> pixel hash failed (but pixel test still passes)
.
svg/batik/text/textStyles.svg -> pixel hash failed (but pixel test still passes)
.
svg/batik/text/verticalText.svg -> pixel hash failed (but pixel test still passes)
.
svg/batik/text/verticalTextOnPath.svg -> pixel hash failed (but pixel test still passes)
.
svg/carto.net ..........
svg/css .
svg/css/arrow-with-shadow.svg -> pixel hash failed (but pixel test still passes)
.
svg/css/circle-in-mask-with-shadow.svg -> pixel test failed
..
svg/css/composite-shadow-example.html -> pixel hash failed (but pixel test still passes)
.
svg/css/composite-shadow-with-opacity.html -> pixel hash failed (but pixel test still passes)
....
svg/css/group-with-shadow.svg -> pixel hash failed (but pixel test still passes)
.
svg/css/mask-with-shadow.svg -> pixel test failed
.
svg/css/path-with-shadow.svg -> pixel hash failed (but pixel test still passes)
.
svg/css/stars-with-shadow.html -> pixel hash failed (but pixel test still passes)
svg/custom .......................................
svg/custom/deep-dynamic-updates.svg -> pixel test failed
...................................................
svg/custom/grayscale-gradient-mask.svg -> pixel test failed
............
svg/custom/image-with-transform-clip-filter.svg -> pixel hash failed (but pixel test still passes)
.................
svg/custom/js-late-mask-and-object-creation.svg -> pixel test failed
.
svg/custom/js-late-mask-creation.svg -> pixel test failed
.
svg/custom/js-late-pattern-and-object-creation.svg -> pixel test failed
.
svg/custom/js-late-pattern-creation.svg -> pixel test failed
............
svg/custom/js-update-pattern-child.svg -> pixel test failed
.
svg/custom/js-update-pattern.svg -> pixel test failed
.......................
svg/custom/marker-changes.svg -> pixel hash failed (but pixel test still passes)
.
svg/custom/marker-child-changes.svg -> pixel hash failed (but pixel test still passes)
......
svg/custom/marker-strokeWidth-changes.svg -> pixel hash failed (but pixel test still passes)
..
svg/custom/mask-changes.svg -> pixel test failed
.
svg/custom/mask-child-changes.svg -> pixel test failed
.
svg/custom/mask-excessive-malloc.svg -> pixel test failed
.
svg/custom/mask-inside-defs.svg -> pixel test failed
.
svg/custom/mask-on-multiple-objects.svg -> pixel test failed
.
svg/custom/mask-with-all-units.svg -> pixel test failed
..............
svg/custom/pattern-cycle-detection.svg -> pixel test failed
.
svg/custom/pattern-deep-referencing.svg -> pixel test failed
.
svg/custom/pattern-in-defs.svg -> pixel test failed
.
svg/custom/pattern-rotate.svg -> pixel test failed
.
svg/custom/pattern-with-transformation.svg -> pixel test failed
.
svg/custom/pattern-y-offset.svg -> pixel test failed
...............
svg/custom/radial-gradient-with-outstanding-focalPoint.svg -> pixel test failed
....
svg/custom/resource-invalidate-on-target-update.svg -> pixel test failed
................
svg/custom/stroked-pattern.svg -> pixel test failed
...........................................................................
svg/custom/use-on-symbol-inside-pattern.svg -> pixel test failed
.........................
svg/custom/visibility-override-mask.svg -> pixel test failed
....
svg/dom .........................
svg/dom/SVGRectElement .
svg/dom/SVGScriptElement ....
svg/dom/SVGStyleElement .
svg/dynamic-updates ................................................................................................................................
svg/filters ..........
svg/filters/filter-clip.svg -> pixel test failed
........
svg/hittest ..
svg/hixie/cascade ..
svg/hixie/data-types ..
svg/hixie/dynamic ......
svg/hixie/error ..................
svg/hixie/links ...
svg/hixie/mixed .........
svg/hixie/painting .
svg/hixie/perf .....
svg/hixie/processing-model ...
svg/hixie/rendering-model .....
svg/hixie/shapes/path .
svg/hixie/text .....
svg/hixie/transform .
svg/hixie/use ...
svg/hixie/viewbox ....
svg/hixie/viewbox/preserveAspectRatio ..
svg/text ....
svg/text/selection-background-color.xhtml -> pixel hash failed (but pixel test still passes)
....
svg/text/text-align-04-b.svg -> pixel hash failed (but pixel test still passes)
....
svg/text/text-deco-01-b.svg -> pixel hash failed (but pixel test still passes)
.......
svg/text/text-path-01-b.svg -> pixel hash failed (but pixel test still passes)
....
svg/text/text-text-01-b.svg -> pixel hash failed (but pixel test still passes)
.
svg/text/text-text-03-b.svg -> pixel hash failed (but pixel test still passes)
.
svg/text/text-text-04-t.svg -> pixel hash failed (but pixel test still passes)
...
svg/text/text-text-07-t.svg -> pixel hash failed (but pixel test still passes)
.
svg/text/text-text-08-b.svg -> pixel hash failed (but pixel test still passes)
.......
svg/transforms ..
svg/transforms/text-with-pattern-inside-transformed-html.xhtml -> pixel test failed
.
svg/transforms/text-with-pattern-with-svg-transform.svg -> pixel test failed
svg/webarchive ...
72.68s total testing time

904 test cases (95%) succeeded
41 test cases (4%) had incorrect layout
1 test case (<1%) had stderr output

[Dimitri Glazkov (Google)]

You can see the failures clearly here:

http://build.chromium.org/buildbot/waterfall.fyi/waterfall?branch=&builder=Webkit+(webkit.org)&builder=Webkit+Linux+(webkit.org)&builder=Webkit+Mac+(webkit.org)&builder=XP+Perf+(webkit.org)&builder=Linux+Perf+(webkit.org)&builder=Webkit+(purify+webkit.org)

[Dimitri Glazkov (Google)]

Just to make sure, with the patch rolled out locally, same command produces:

944 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
1 test case (<1%) had stderr output

[Nikolas Zimmermann]

Comment on attachment 46243 [details]
Bad DOM performance in large SVG files

Clearing review flag, this patch needs to be revisited. Anyone who works on this has to be aware that regressions can only be spotted when running _PIXEL TESTS_.

[Nikolas Zimmermann]

Comment on attachment 46244 [details]
Bad DOM performance in large SVG files

Taking back review, as discussed with Oliver.

[Oliver Hunt]

Created attachment 46667 [details]
Patch

[Oliver Hunt]

Created attachment 46668 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 46668 [details]
Patch

Okay, let's hope it fixes it this time... Can you file a follow-up bug regarding PaintInfo fixes is renderSubtreeToImage?

[Oliver Hunt]

Committed r53331: <http://trac.webkit.org/changeset/53331>

[Dimitri Glazkov (Google)]

Darnit! One test still broke: http://trac.webkit.org/browser/trunk/LayoutTests/fast/borders/svg-as-border-image-2.html

Can you take a look and see what's happening there?

[Dimitri Glazkov (Google)]

Rolled out Oliver's patch in http://trac.webkit.org/changeset/53334, per IRC discussion w/Nikolas.

But you're really, really close now! Don't give up!

[Oliver Hunt]

Created attachment 46701 [details]
Patch

[Oliver Hunt]

Committed r53343: <http://trac.webkit.org/changeset/53343>

[Oliver Hunt]

Created attachment 46715 [details]
Patch

[Oliver Hunt]

Just opening for a improvement to culling

[Sam Weinig]

Comment on attachment 46715 [details]
Patch

Please add more complete explanation to the ChangeLog.  r=me

[Oliver Hunt]

Committed r53349: <http://trac.webkit.org/changeset/53349>