Closed Bug 801949 Opened 12 years ago Closed 1 year ago

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

Categories

(Core :: SVG, defect)

17 Branch
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox17 - ---
firefox18 - ---
firefox19 - ---

People

(Reporter: jwatt, Assigned: jwatt)

References

(Depends on 1 open bug, )

Details

(Keywords: perf, regression, Whiteboard: [invalidation] [in-the-wild])

+++ 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.
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
A lot of the slowness I'm seeing here on m-c is due to bug 801918.
Depends on: 801918
Another source I've isolated is bug 802321.
Depends on: 802321
Keywords: perf
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.)
Oh, and the regression from comment 1 is not an invalidation issue. (Sadly, since that would make it easier to identify the fix.)
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.
Untracking for 17, Jwatt can you give a status update and eta on a fix for 18?
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.
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.
Depends on: 802440
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.
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: Scrolling slowdown and hang on SVG demo → Scrolling has become jerky on SVGs containing a large number of elements (jank)
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.)
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.
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.
(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.
(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.
Fixing bug 828240 might help here, but probably not by all that much.
Depends on: 828240
Depends on: 869505
Whiteboard: [jwatt:invalidation]
Whiteboard: [jwatt:invalidation] → [jwatt:invalidation] [in-the-wild]
Depends on: 1016525
Performance improved dramatically when I last checked it two weeks ago. It appears to have regressed again.
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.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
(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 ...
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Status: REOPENED → NEW
No longer depends on: 828240
Summary: Scrolling has become jerky on SVGs containing a large number of elements (jank) → Over invalidation when scrolling SVG containing a large number of elements (jank)
Whiteboard: [jwatt:invalidation] [in-the-wild] → [invalidation] [in-the-wild]
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.
Depends on: 1352499
Depends on: 1446667
Severity: normal → S3

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.

Flags: needinfo?(jwatt)

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.

Flags: needinfo?(jwatt)

Seems fine now, presumably because we retain display lists.

Status: NEW → RESOLVED
Closed: 8 years ago1 year ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.