801949 - Over invalidation when scrolling SVG containing a large number of elements (jank)

Status: RESOLVED WORKSFORME

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

+++ This bug was initially created as a clone of Bug #800198 +++

STR:
1) Open this SVG demo: http://www.jasondavies.com/factorisation-diagrams/
2) Scroll the page with the vertical scrollbar or the middle button

Result: 
Scrolling is slowed down and isn't as smooth as in FF16. After scrolling the page during a while, f you switch to another tab then you come back, there is a slight hang to display the tab with the SVG demo.

m-c
good=2012-08-02
bad=2012-08-03
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=588424024294&tochange=89dcadd42ec4

There are some bugs about SVG patches, like bug 655877 or bug 776054.

Comment 1

[Jonathan Watt [:jwatt]]

Alice commented in bug 800198 comment 1:

Regression window(cached m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a799b5bff84c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120801155239
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b077c43a4306
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120801163038
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a799b5bff84c&tochange=b077c43a4306

And svg.display-lists.painting.enabled = false helps

Comment 2

[Jonathan Watt [:jwatt]]

A lot of the slowness I'm seeing here on m-c is due to bug 801918.

Comment 3

[Jonathan Watt [:jwatt]]

Another source I've isolated is bug 802321.

Comment 4

[Jonathan Watt [:jwatt]]

I had to rebuild my Windows machine over the weekend, but I now have a Windows build environment and I _can_ see scrolling get a little clunkier over the regression range given in comment 1 on Windows. That said, that slowdown is completely dwarfed by the DLBI issues that I've been isolating from this bug over the last few days (bug 801918, bug 802440, the issue fixed in bug 800198, and the two issues fixed in bug 802321). I'm guessing tracking was granted based on the much larger slowdown caused by these issues, so we may want to revisit the decision to track after they're fixed. (Which is to say that, while we definitely want to fix the regression identified in comment 1, that issue may not be severe enough to warrant tracking.)

Comment 5

[Jonathan Watt [:jwatt]]

Oh, and the regression from comment 1 is not an invalidation issue. (Sadly, since that would make it easier to identify the fix.)

Comment 6

[Alex Keybl [:akeybl]]

Unless we think this will cause major web regressions, we're very likely to untrack for FF17/18. Please let us know if that's a problem.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Untracking for 17, Jwatt can you give a status update and eta on a fix for 18?

Comment 8

[Jonathan Watt [:jwatt]]

There's not much point in me fixing this for 18 until bug 801918 is fixed since that issue is much more severe than the original. Even if that was fixed now, I've no idea when I'll get to this since I have plenty of other higher priority bugs assigned to me right now.

Comment 9

[Alex Keybl [:akeybl]]

Yep, this isn't the end of the world and we don't know of any major affected web properties. No need to track for upcoming releases at all in that case - glad we agree.

Comment 10

[Jonathan Watt [:jwatt]]

Despite bug 800198, bug 801918, bug 802321 and bug 802440 now being fixed I'm still seeing frequent full page invalidation when scrolling the reported page. I'm also seeing weird smaller invalidations around the fringes of the SVG blocks while they're in the center of the page while scrolling.

Comment 11

[Jonathan Watt [:jwatt]]

It seems like the majority of the time is being spent under ContainerState::ProcessDisplayItems and BuildDisplayList. The JS in the demo generates a very large number of SVG elements, and the result is that we end up creating a very large number of display list items.

One of the concerns I voiced prior to converting SVG to use display lists was that more complex SVGs would take a perf hit from the overhead of creating and handling the number of display list items they'd require (SVGs are more likely to contain many, many more onscreen elements than HTML is). Maybe someone who understands layer generation better can see a bug in ContainerState::ProcessDisplayItems that might be causing an issue here, but otherwise, on Windows at least, I think that's largely the problem that this SVG demo is encountering. (On Mac there's still the frequent full page invalidation thing going on too.)

Comment 12

[Chris Lord [:cwiiis]]

jwatt and I profiled this just now - the invalidation looks ok now, at least in Nightly on Windows (I've not tried elsewhere), but the profile showed an inordinate amount of time being spent in ProcessDisplayItems.

Although drawing is high in the profile, resizing the window to be the same width horizontally, but very narrow vertically, the frame-rate improves markedly. This goes some way to pointing towards display list processing being the problem, as having less visible on the screen would reduce the size of the display list a lot.

Although this is probably a pathological case (many, complex SVGs, visible at once), this is of some concern to mobile. On mobile, we render a larger area than the screen, and an even larger area than that (by a factor of 4) when rendering is deemed to not be keeping up. Polynomial display list processing costs would not be desirable in that case.

I have no clever ideas of how to fix this just now... I expect we want some kind of caching of the work that is done in ProcessDisplayItems so that we can avoid spending a long time on branches of the display list that never change.

Comment 13

[Chris Lord [:cwiiis]]

Just to add some quantities to comment #11 and comment #12, taking a profile of scrolling up and down on my machine and taking the time in nsRefreshDriver::Tick:

gfx::DrawThebesLayer - 46.7% (mostly spent in paint and fill)
ContainerState::ProcessDisplayItems - 33.9%
ViewportFrame::BuildDisplayList - 10.1%
nsDisplayList::ComputeVisibilityForRoot - 5.1%

Nothing else shows up in a way that's worth mentioning.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Chris Lord [:cwiiis] from comment #12)
> Polynomial display list
> processing costs would not be desirable in that case.

The processing time should be linear in the number of visible items, not super-linear.

Comment 15

[Chris Lord [:cwiiis]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
> (In reply to Chris Lord [:cwiiis] from comment #12)
> > Polynomial display list
> > processing costs would not be desirable in that case.
> 
> The processing time should be linear in the number of visible items, not
> super-linear.

My bad, that's good to know then. That makes this less worrying, though I suppose it'd still be good to cache something somewhere to avoid unnecessary work. He says, vaguely.

Comment 16

[Jonathan Watt [:jwatt]]

Fixing bug 828240 might help here, but probably not by all that much.

Comment 18

[mcelam]

Here's a URL for some extreme levels of SVG-induced jank.

  http://bl.ocks.org/stepheneb/1296930

Complete UI freeze for a second or two when scrolling over the generated image. Does not occur when the SVG is scrolled up off screen. Switching tabs also shows a marked speed problem when clicking back to the above URL, but only if the SVG is visible in the viewport.

I am on OS X 10.9 and using up-to-date Nightly.

Comment 19

[mileselam]

Performance improved dramatically when I last checked it two weeks ago. It appears to have regressed again.

Comment 20

[Justin [:JW_SoftvisionQA]]

I can not reproduce this issue on Mozilla/5.0 (Windows NT 6.3; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

I will close this bug. Please feel free to reopen if the bug persists.

Comment 21

[(mostly gone) XtC4UaLL [:xtc4uall]]

(In reply to Justin [:JW_SoftvisionQA] from comment #20)
> Please feel free to reopen if the bug persists.

Indeed it does, just compare current release (non-e10s) and trunk (e10s) with/against ESR 10 - difference like day and night ...

Comment 22

[Jonathan Watt [:jwatt]]

Having the layout.display-list.retain pref set to true makes a noticeable difference when scrolling quickly from the top to the bottom of the page.

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 13 votes.
:jwatt, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 24

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 25

[Robert Longson [:longsonr]]

Seems fine now, presumably because we retain display lists.