Closed Bug 1507508 Opened 7 years ago Closed 4 years ago

Adding markers to a Google map is visibly slower than in IE, Chrome, etc.

Categories

(Core :: Performance: Responsiveness, defect, P3)

63 Branch
defect

Tracking

()

RESOLVED INVALID
Performance Impact medium
Tracking Status
firefox-esr60 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix

People

(Reporter: JeremyHussell, Assigned: jesup)

Details

(Keywords: perf, perf:responsiveness)

Attachments

(4 files, 2 obsolete files)

Attached file markerPerformance.html
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:63.0) Gecko/20100101 Firefox/63.0 Steps to reproduce: Open the attached file in Firefox. Actual results: Testing using Firefox 63 on Windows 7, I can see markers being added one at a time over the course of ~4s. (Try increasing the number of markers if you're using a beefy machine.) Testing using IE 11, Edge, and Chrome on Windows 7, the markers are added in <1s. In the test file, replace the section: icon: { fillColor: '#ffff00', fillOpacity: 1, path: google.maps.SymbolPath.CIRCLE, scale: 3.5, strokeWeight: 1, }, With this: icon: { anchor: { x: 4, y: 4 }, url: 'yellowCircle.svg', }, And save this as "yellowCircle.svg": <svg height="8" width="8" viewBox="0 0 8 8" xmlns="http://www.w3.org/2000/svg"> <circle cx="4" cy="4" r="3.5" fill="#ff0" stroke="#000" /> </svg> Now Firefox adds the markers in roughly the same amount of time as the other browsers. This is just a different way of telling Firefox to draw the same circle. (From my point of view. I have no idea what's going on under the hood.) Expected results: Markers should be added to the map at roughly the same (fastest reasonable) speed no matter how they're specified. The time to add markers should be roughly the same as for other browsers. Impact: makes adding 1000 markers to a map take >1s, which has important usability implications (for this type of map, on Firefox). (I work with this type of map.) This probably also generates a lot of bug reports directed to the Google Maps team, which must be frustrating for them.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
Component: General → Performance
Profile: (linux64, non-WR, Inbound from a day ago with a JIT Shape counter patch by mgaudet added; shouldn't affect this) https://perfht.ml/2PAPnX1 Lots of hits in recvmsg(); almost all related to layers (RenderLayerWithCallback, etc): https://perfht.ml/2QO6wt0 Overall a lot of time in Paint/etc functions Lots of Reflows; starting around 0.5ms, running up to 2.5ish ms by the end. Lots of DisplayLists (see marker chart); starting at a few ms and going up to 20ms(!) by the end. First non-blank paint after 1.1s; first contentful paint after ~2.4s Layerbuilding starts out in low-ms range; goes to ~10ms by the end. LOTS of load* DOM events, and MozAfterPaint events - and each load* (load/loadstart/loadend) seems paired with a securitypolicyviolation event. Closeup of a section: https://perfht.ml/2QLBfqp Quite a lot in or under google maps API code (~1.8s of 8.9s sampled under _n.vh()) I don't know enough about how CSS (or SVG/GFX) is handled to comment on why the alternative is faster. I'll take an alternative profile
Attached file markerPerformance.html (modified) (obsolete) —
(modified file) Shockingly faster.... https://perfht.ml/2QNjmHX
Attached image yellowCircle.svg
Attachment #9025702 - Attachment description: markerPerformance.html → markerPerformance.html (modified)
One ton less DOM events due to the change (though they're not really the problem, they're a symptom/marker)
Overall difference is around 8s (for correct comparison, this is the original loaded from /tmp: https://perfht.ml/2QSyFiJ) vs 2.2s -- and the first 1.5-2 seconds of the load is the same basically for both.
Attachment #9025702 - Attachment is obsolete: true
There seems to be one major difference between these two ways of adding the markers: with the "slow" way, the mouse cursor changes to a pointer ("link click hand") when you hover the circle, and with the "fast" way, the mouse cursor doesn't change over the circles. The changing of the mouse cursor is achieved by inserting the following HTML for each marker: <div style="width: 8px; height: 8px; overflow: hidden; position: absolute; opacity: 0; cursor: pointer; touch-action: none; left: -105px; top: 89px; z-index: 93;" title=""><img style="position: absolute; left: 0px; top: 0px; width: 8px; height: 8px; -moz-user-select: none; border: 0px none; padding: 0px; margin: 0px; max-width: none;" alt="" src="https://maps.gstatic.com/mapfiles/transparent.png" draggable="false"></div> That's a zero-opacity div, with a transparent image nested inside it. In the modified testcase, these divs and imgs are not present. The profile shows that in the slow testcase, the insertion of these elements is interleaved with calls to canvas.getContext(), which flushes layout in Firefox. It's this amount of layout thrashing that makes this so slow. The other thing that makes it slow is that these elements get inserted from a setTimeout callback, which allows Firefox to paint in between those setTimeout callbacks; that's why you see the markers appear over many different paints.
So what's the difference between us and Chrome (and Edge?) here? Is it the layout flush? Or is it due to painting between the timeouts? I should take a profile in Chrome...
For me, the cursor changes to a pointer when hovering over a circle no matter which method is used, whether in Firefox, Edge, or Chrome. From the user's perspective there's no functional difference. There's still a difference in loading times in Edge and Chrome, but it's much smaller than the difference in Firefox.
Oh, indeed! I had messed with the DOM in the devtools inspector in a way that broke the script on the page. If I load attachment 9025717 [details] unmodified, I get correct mouse hover behavior. But it is implemented in a different way: The cursor property is updated dynamically on a large wrapping element from a mousemove event handler whenever the mouse is over a marker. This: <div style="position: absolute; z-index: 0; left: 0px; top: 0px; height: 100%; width: 100%; padding: 0px; border-width: 0px; margin: 0px; cursor: url(&quot;https://maps.gstatic.com/mapfiles/openhand_8_8.cur&quot;), default; touch-action: none;" tabindex="0"> changes to this: <div style="position: absolute; z-index: 0; left: 0px; top: 0px; height: 100%; width: 100%; padding: 0px; border-width: 0px; margin: 0px; cursor: pointer; touch-action: none;" tabindex="0"> (note the cursor: pointer in there) So anyway, that explains why the performance between the two testcases is different: they do very different things. However, the fact that the first testcase is so much slower than in other browsers is still a valid bug.
Priority: -- → P3
Whiteboard: [qf]

Daniel, thoughts on who should look at this?

Flags: needinfo?(dholbert)
Whiteboard: [qf] → [qf:p2:responsiveness]

I don't know without investigating, so I just went ahead and investigated.

Focusing on the original testcase: it looks like the difference between us and Chrome there is as follows: Firefox yields for a refresh-driver-tick (and repaint) between consecutive setTimeout callbacks, whereas Chrome does not. I'll attach a testcase that demonstrates this.

This behavior-difference just so happens to benefit Chrome in this scenario, where they have manymany setTimeout calls queued up, each of which triggers a non-trivial chunk of styling/layout/painting if it's given an opportunity right away. Chrome doesn't yield & hence gets to coalesce the effects of all of those setTimeout callbacks, while we yield for a refresh driver tick between the setTimeout callbacks. In general (not on this specific testcase), this means we tend to seem more responsive (repainting & handling user input more frequently), but Chrome will reach its final rendering a bit faster (due to skipping over intermediate renderings).

Flags: needinfo?(dholbert)

Here's a very simple testcase which I think is effectively the same as this bug (though in this case the repaints are cheaper).

Firefox yields for a repaint between each queued-up setTimeout callback here, so we show each value of the incrementing counter. But Chrome does not -- they don't paint any intermediate states, just the final value.

smaug, do you know if we've got other bugs open about this behavior-difference between Firefox and Chrome? (firefox yielding for refresh driver ticks between previously-queued-up setTimeout invocations, vs. Chrome not yielding)

Does this belong in DOM perhaps?

Flags: needinfo?(bugs)

oh, interesting. Chrome janks badly.

We do have pref dom.timeout.max_consecutive_callbacks_ms, and increasing that a lot changes the behavior on the testcase. (I tested 10000, which is silly).
Does increasing dom.timeout.max_consecutive_callbacks_ms significantly affect Google Maps?

Flags: needinfo?(bugs)

Thanks - yes, if I make that pref adjustment, then the original Google Maps testcase here loads as-expected (the same way it loads in Chrome). There are only a couple flashes of painted content, rather than many many "per-marker" repaints, and we reach the final rendering sooner.

Here's a profile with that pref-tweak: https://perfht.ml/2YyVLxU
vs. a profile with default pref value: https://perfht.ml/2YvZ7Sn

I noticed this too browsing google maps. Occasionally when I zoom in and out to see different portions of the map, the zoom out view will be stuck with blank content on parts of the screen for a long time (13 seconds)
https://perfht.ml/2WQDLia

Changing the pref to 1000, I can get a split second of blank content but it usually resolves quickly.

So other than saying "Don't do that!", are there any options here?

One option would be to treat 0ms SetTimeouts differently than others.... don't apply the same max_consecutive_callbacks for string of 0ms timeouts waiting to fire (use a higher-but-not-infinite limit perhaps). 0ms timeouts aren't really "timeouts", they're "let other events in" or a form of continuation programming

Hmmm. Vicky, looking at your profile (if it caught the freeze), I don't see a barrage of timeouts: https://perfht.ml/2HuGKGE
and https://perfht.ml/2HsjIjK

I suspect whatever is blocking you is not on MainThread... a worker? There are no DOM events really until it "comes alive" again (https://perfht.ml/2Hy4CZZ)

This zoom in on one of the few DOM events in the freeze is interesting... https://perfht.ml/2HC4ngK MozLocalStorageChanged

perhaps profile workers and Quota (LSNG maybe?)

Flags: needinfo?(vchin)

Here's another profile with more threads captured:
https://perfht.ml/2WNCN6s

Flags: needinfo?(vchin)

Yeah, setTimeout(, 0) could be perhaps treated differently. But still, painting should happen between those, even the spec seems to hint about that.
This does feel like Google maps is relying on jank-y behavior in Chrome.

(In reply to Olli Pettay [:smaug] from comment #22)

Yeah, setTimeout(, 0) could be perhaps treated differently. But still, painting should happen between those, even the spec seems to hint about that.

You can make a real argument that setTimeout(0) is expected to do different things than "normal" timeouts.

This does feel like Google maps is relying on jank-y behavior in Chrome.

We can poke them if we have contacts on the Maps team

Mike?

Flags: needinfo?(miket)

Yeah, setTimeout(, 0) could be perhaps treated differently. But still, painting should happen between those, even the spec seems to hint about that.

You can make a real argument that setTimeout(0) is expected to do different things than "normal" timeouts.

I did an experimental patch that allows sequences of 0ms timers to run for longer (25ms instead of 4ms) before exiting the timeout loop. This only applies to 0ms timers, and only for so long as the entire set being run in RunTimeout are 0ms (though it doesn't check before running a timeout, only after - I could do that, and move the zero-ms/"over timeout" check before the running the each timeout instead of after.

Net result: ~4-4.5s vs 7.5s. Chrome on this machine is around 2-2.5s. Setting it to effectively infinite causes it to take 3.5s. Note that we still kick out and allow gfx to start painting if a non-0 one gets in there

smaug: thoughts?

Flags: needinfo?(bugs)

mike - after discussion and some experiments: if they (google) want that (janky) behavior when putting a ton of pins in (i.e. don't paint much if at all until done), they can use resolved promises (i.e. microtasks, which happen before paint). If they want to limit the duration of jank in that impl, they can limit the number/time-spent of promises before spinning the event loop via setTimeout(..., 0).

I suspect this may be an edge-case they don't care about much, as it's not main Google Maps app; it's using the API.

Sorry for dropping the ball on this one. Does this still reproduce as described? For me, the performance appears to be the same in Firefox and Chrome now (though, just visually, not measuring).

Flags: needinfo?(miket) → needinfo?(JeremyHussell)

Yes, it still reproduces as described on the latest versions of Firefox and Chrome. (https://bugzilla.mozilla.org/attachment.cgi?id=9025377) From previous discussion, it seems the Google Maps team is taking advantage of a bug in Chrome (accidentally?), and not actually a bug in Firefox, which I presume is why everyone stopped paying attention to this issue.

Flags: needinfo?(JeremyHussell)
Assignee: nobody → rjesup
Status: NEW → ASSIGNED

Is this still an issue?

(I tried the initial testcase and it caused Chrome (parent process!) to hang every now an then)

Flags: needinfo?(bugs)

Testing on the latest versions of Firefox and Chrome, the behaviour is now indistinguishable. Unfortunately - from my point of view - what changed is that the timer bug in Chrome has been fixed, so the example is now just as slow on Chrome as it is on Firefox. Now all that's left is a performance issue for the Google Maps API team to deal with.

Thank you, everyone who helped track down the cause of this issue. Even though I'm still annoyed by the enormous difference in performance between adding the same markers in different ways, I acknowledge that making Firefox bug-for-bug compatible with Chrome would have been the wrong thing to do. With the Chrome bug fixed, the Maps people will, hopefully, be pushed to change their code to add markers quickly no matter which browser is being used.

Thanks for confirming that! Sounds like this bug can be closed, then.

I'll close this as INVALID, in the sense that there-is-no-Firefox-bug-to-be-fixed-here. (To the extent that a bug exists, it's a perf bug on the Google Maps side, which [now] affects all browsers, as best we can tell.)

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
Attachment #9065689 - Attachment description: Bug 1507508: experiment with letting strings of 0ms timers run for extra time f=smaug → Bug 1507508: experiment with letting strings of 0ms timers run for extra time r=smaug
Attachment #9065689 - Attachment is obsolete: true
Performance Impact: --- → P2
Whiteboard: [qf:p2:responsiveness]
Component: Performance: General → Performance: Responsiveness
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: