Open Bug 837985 Opened 7 years ago Updated Last year

OpenStreetMap's new javascript-based editor iD is sluggish

Categories

(Core :: Web Painting, defect)

x86
All
defect
Not set

Tracking

()

People

(Reporter: nirvn.asia, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: perf, Whiteboard: [jwatt:invalidation])

OpenStreetMap's new javascript-based editor iD is sluggish under Firefox (tested with current nightly) while performs smoothly on Chromium on my relatively powerful desktop machine (Core i7 3.2GHz; NVIDIA GeForce GTX 260).

iD website: http://ideditor.com/
iD demo: http://geowiki.com/iD/

I wish I could do more to dissect the problem. Hopefully this bug will fall onto the radar of someone who can. 

(It's been filed under the General component as I don't know what is causing the poor performance. Could be SVG related, but that's a wild guess)
Mathieu, what are the steps to reproduce on, say, http://geowiki.com/iD/ ?  As in, where should I click or whatnot to reproduce the sluggishness?

Also, just to make sure, you're seeing this on all platforms?  Or just specific ones (which ones)?
Boris, right.

Few steps to witness Firefox under-performing:
1. Open http://geowiki.com/iD/
2. Zoom out once (by clicking on the [ - ] icon on the left side)
3. Click and hold your mouse's left button down to drag the map around (try to go north, south, east, west, and repeat this motion a couple of times)

Alternatively:
1. Open http://geowiki.com/iD/
2. Zoom out once (by clicking on the [ - ] icon on the left side)
3. Click and hold your mouse's left button down over a polygon (or line) node, and move that node around in circles

By that stage, you should have ran into the "sluggish" behavior.

Regarding platforms, the behavior is present both on the windows and linux platform
Confirmed against Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130205 Firefox/21.0 ID:20130205031033

Profile with moving the Map around:
http://people.mozilla.com/~bgirard/cleopatra/#report=4b5542aa351574c1ced04de4453dc4c324c5106b

Profile with moving a Node around:
http://people.mozilla.com/~bgirard/cleopatra/#report=ccb6a4ffecf56fec0fc983c5c9356f7064219e44

FWIW, there have been filed other Reports against D3.js Library Perf Issues already.
Keywords: perf
Thanks for the steps to reproduce.

The vast majority of the time I see taken when I profile is painting (80+% of the time).
Component: General → Layout: View Rendering
I see something else when I focus on a janky interval (janky according to SPS...) --- not painting. Here's my profile: http://people.mozilla.com/~bgirard/cleopatra/#report=88ff232a7625db12b8d0ca153f861eea54b311d4 ... I'm looking at sample range [1739,2880].

It looks like lots of mouse move events (31+) being processed in a row with no painting in between. 86% of the profile is under FlushPendingNotifications(Flush_InterruptibleLayout). About half of that is reresolving style contexts, and much of that is under nsSVGPathGeometryFrame::DidSetStyleContext doing nsSVGUtils::InvalidateBounds --- sounds like bug 827915.

The other half of FlushPendingNotifications is doing reflow. It's SVG frames being reflowed, which is how we update overflow areas on them.

The FlushPendingNotifications is being called via FlushPendingEvents in nsEventStateManager::PreHandleEvent. I don't think that should really be there though...
Depends on: 827915
One thing is that we should be seeing more event coalescing than that. Ideally we shouldn't have more than one mouse-move event per refresh driver tick. The rest is just wasted work.
In particular I thought the decisions about flushing mouse events during event handling were made in nsPresShellEventCB during event handling, instead of in ESM. The former is a much more logical place to make them, I think.
I think probably we should focus on the mouse-move event flooding. If we just fix that, this bug would be much better. I'm not 100% sure *how* to fix it though.

Would it make sense to allow at most one mouse event to be processed in a window between each refresh driver tick? I.e. if we receive a mouse-move event, and we haven't had a refresh driver tick since the last mouse-move event, then schedule a refresh driver tick and defer the mouse-move event handling to happen after the refresh driver tick --- with the latest mouse coordinates of course.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> One thing is that we should be seeing more event coalescing than that.
> Ideally we shouldn't have more than one mouse-move event per refresh driver
> tick. The rest is just wasted work.
I'm not quite sure about this. Some games for example might want to get as exact input as possible,
even if they can't paint faster than what refresh driver runs.

But maybe for mousemoves...click handling just gets a bit more difficult since we really should
get mouseup and click asap, and if there are pending mousemoves we'd need to process those first.
(In reply to Olli Pettay [:smaug] from comment #9)
> I'm not quite sure about this. Some games for example might want to get as
> exact input as possible,
> even if they can't paint faster than what refresh driver runs.

Hmm.

> But maybe for mousemoves...click handling just gets a bit more difficult
> since we really should
> get mouseup and click asap, and if there are pending mousemoves we'd need to
> process those first.

We don't need to deliver intervening mousemoves, we could just drop them and do the mouseup and click right away.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5)
> The FlushPendingNotifications is being called via FlushPendingEvents in
> nsEventStateManager::PreHandleEvent. I don't think that should really be
> there though...

Once I patch that out, this page is jank-free for me with accelerated (D2D) rendering on Windows. Still kinda janky on with D2D disabled, and that's painting. So I think we have two separate problems here: mouse-move flooding with associated flushes, and CPU-bound painting.
roc, glad to hear there's progress. This is bound to become the main OSM editor in some not too distant future, good to know firefox won't under-perform.

While stopping mouse-move flooding alleviate the slowness, there's definitively a SVG-related painting issue that affects more than this site. I raised a different (but possibly related) issue some time ago (bug 788093) about Firefox under-performing when rendering SVG polygons in comparison other browsers. Wondering whether there's a similar root cause for the painting issue.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5)
> The FlushPendingNotifications is being called via FlushPendingEvents in
> nsEventStateManager::PreHandleEvent. I don't think that should really be
> there though...

Taking that out would probably regress bug 716549. So we really need to throttle mousemoves, I think.
OpenStreetMap officially launched this editor for the masses on May 7.
(In reply to Mathieu Pellerin from comment #14)
> OpenStreetMap officially launched this editor for the masses on May 7.

Which means this is now the default editor on their home page and therefore a lot of people trying the primary path to edit maps will run into the problems described here.
It's not actually the default yet - some of the press materials suggested it would be but that was due to various misunderstandings.

Right now it is available as an option but is not default - one of the main reasons I am reluctant to make it the default at the moment is the performance in Firefox.
iD developer here. Thanks for looking into this on the Firefox side. If your work leads you to any conclusions about changes that could be made on the SVG/CSS/JS side to improve performance, I'd be eager to try them out.
John, throttling mousemove handlers in the page script to not run too often might help here.
I removed all mousemove handlers except the one implementing the pan behavior, and then tried throttling that to 10, 20, and 30 ms. Removing the non-essential handlers seemed to help somewhat, but the pan animation was still janky with any throttle (and of course, throttling to more than 33 ms is going to naturally produce jank).
I've enabled layers.acceleration.draw-fps and tested disabling a variety of different features -- opacity, hover effects, background imagery, all map CSS other than basic strokes. So far, the only thing that gets me above 10 FPS during a pan operation in a dense area is brute reduction of the number of SVG elements that are rendered.

Are there any other tools to help diagnose paint performance problems?
Here's a reduction:

http://bl.ocks.org/jfirebaugh/raw/5580143/

Chrome gets a consistent 60 FPS, Firefox < 1 FPS (appears to be redrawing the entire SVG element on every paint).
The dragging in that example is going to be slow because setting the CSS transform will cause us to hit bug 854765 and invalidate and repaint all the SVG. I'm working on that, but even once that issue is fixed the initial paint is going to take a long time since putting a million SVG elements on screen is going to hit bug 869505.
Depends on: 854765, 869505
It looks like the mouse event flooding problem I was seeing before has gone away (something in the mousemove JS event handler has stopped flushing layout, by the look of it) and now comment #22 describes the problem.
FYI, http://openstreetmap.us/iD/master/ is a deployment with the lastest code, including some JS performance improvements. Not seeing many more opportunities on that front. Unless there's some other way to translate an SVG element that doesn't trigger a full invalidation, bug 854765 is the primary limiter of performance on Firefox at this point.
Whiteboard: [jwatt:invalidation]
(In reply to John Firebaugh from comment #24)
> FYI, http://openstreetmap.us/iD/master/ is a deployment with the lastest
> code, including some JS performance improvements. Not seeing many more
> opportunities on that front. Unless there's some other way to translate an
> SVG element that doesn't trigger a full invalidation, bug 854765 is the
> primary limiter of performance on Firefox at this point.

Well, the patch for that bug has landed on Nightly now, so would be interesting if you can see this helping (you can get a Nightly build from nightly.mozilla.org for testing). If that's the case, then Firefox 24 will help there. Otherwise, I guess we need to wait for bug 869505 work.
I've been testing with Nightlies, and subjectively there has been little to no improvement. It's still sluggish overall on the same key operations: panning the map, zooming, and selecting map features.
Same observation here, cannot see tangible decrease in sluggishness when using iD editor.
Firefox 24 give some SVG feature: https://bugzilla.mozilla.org/show_bug.cgi?id=600207 I didn't try it before, it's look less suggly.

I hope this bug will be fixed soon. Mozilla push to use web application on HTML 5 with Firefox OS and this OSM editor iD is for me a good example.
Blocks: 934525
Has there been any progress on this issue? It still seems to be a problem
Bug 869505 blocks this one, and not much activity there, unfortunately.
(In reply to Harry Chapman from comment #30)
> Has there been any progress on this issue? It still seems to be a problem

To day, in 1 hour, I kill 6 times Fx for edit a little change in my city... View the changes is just incredibly slow.
Related: bug 1187850, which has a reduced testcase and some comments from roc.

Gerv
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.