1421885 - Google Maps map with many pins is very slow to load pins

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P1)

Description

[Asif Youssuff]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20171129220149

Steps to reproduce:

1. Navigate to: http://suffolkcamra.co.uk/pubs/cgi-bin/demomap.cgi


Actual results:

Firefox takes about 25 seconds to load all pins on map. 


Expected results:

Closer to Chrome's 5-6 seconds. 

Performance profile: https://perfht.ml/2BxDXbS

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

From the profile, it seems in the 36s range of content 1 process work, 30s is under PresShell::Paint, so it sounds like something slow is happening in painting.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Painting is pretty slow here, and there's lot to investigate there, but it seems like the main issue is that we're doing only a few markers each paint.

I had a look with Chrome and the paints are super slow there too (maybe worse even), but we only paint 2-3 times to add the entirety of the markers.

It seems like there's some sort of scheduling difference here that means we do way less work adding markers for each paint.

Smaug, do you know if there's anything intentional/known related to this? I haven't debugged it much yet, but it looked like it was using setTimout (TimeoutManager::RunTimeout in the profiler).

It's quite hard to figure out what the google maps API is doing (all minified source).

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1421885 - Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion.

Review commit: https://reviewboard.mozilla.org/r/255722/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255722/

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1421885 - Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures.

Review commit: https://reviewboard.mozilla.org/r/255724/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255724/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8990645 [details]
Bug 1421885 - Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion.

https://reviewboard.mozilla.org/r/255722/#review262500

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8990646 [details]
Bug 1421885 - Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures.

https://reviewboard.mozilla.org/r/255724/#review262502

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f106d66d06e
Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion. r=kats
https://hg.mozilla.org/integration/autoland/rev/42f32a0e7508
Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures. r=kats

Comment 8

[Andrei Ciure[:aciure]]

Backed out 2 changesets (bug 1421885) for helper_touch_action_complex.html failures

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=42f32a0e75082445bf9edc7707c71f42c29c560b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=187240176&filter-searchStr=Linux+x64+debug+Mochitests+with+e10s+test-linux64%2Fdebug-mochitest-plain-headless-e10s-13+M-e10s%28h13%29

log: https://treeherder.mozilla.org/logviewer.html#?job_id=187240176&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/cd0eb8dbd0861e34f41e7520dbd21806565c14f1

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8990645 [details]
Bug 1421885 - Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255722/diff/1-2/

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8990646 [details]
Bug 1421885 - Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255724/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cac0b0aa948e
Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion. r=kats
https://hg.mozilla.org/integration/autoland/rev/faab566585a2
Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures. r=kats

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50526e5ade88
Backed out 2 changesets for test_group_touchevents-3.html failures CLOSED TREE

Comment 13

[Andrei Ciure[:aciure]]

Backed out 2 changesets (bug 1421885) for test_group_touchevents-3.html failures

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=faab566585a2184c4d024506d0672da4e77e95b5&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=187275787&filter-searchStr=Linux+x64+asan+Mochitests+with+e10s+test-linux64-asan%2Fopt-mochitest-e10s-8+M-e10s%288%29

log: https://treeherder.mozilla.org/logviewer.html#?job_id=187275787&repo=autoland

backout: https://hg.mozilla.org/integration/autoland/rev/50526e5ade88588e3924c006c743834c90ffdff3

Comment 14

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8990645 [details]
Bug 1421885 - Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255722/diff/2-3/

Comment 15

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8990646 [details]
Bug 1421885 - Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255724/diff/2-3/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b01200eb6ad4
Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion. r=kats
https://hg.mozilla.org/integration/autoland/rev/0c55071115c2
Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures. r=kats

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/b01200eb6ad4
https://hg.mozilla.org/mozilla-central/rev/0c55071115c2

Comment 18

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8990645 [details]
Bug 1421885 - Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion.

Approval Request Comment
[Feature/Bug causing the regression]: Fixes bug 1487890
[User impact if declined]:
[Is this code covered by automated tests?]: Covered by the APZ test/
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just adds some region simplification calls to improve performance, and fixes a bug in the process.
[String changes made/needed]: None.

Comment 19

[Julien Cristau [:jcristau]]

Comment on attachment 8990646 [details]
Bug 1421885 - Part 2: Don't compute APZ touch-action regions on platforms that don't support touch gestures.

see comment 18.

Comment 20

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 59.0a1 (BuildId:20171129220149) on Windows 10 64bit.

This issue is verified fixed using Firefox 63.0b7 (BuildId:20180917143811) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit.

Comment 21

[Julien Cristau [:jcristau]]

Comment on attachment 8990645 [details]
Bug 1421885 - Part 1: Don't let mNoActionRegion get complex when we've already added it to mDispatchToContentRegion.

fixing bug 1487890, approved for 62.0.2

Comment 22

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/e2b254faaebb
https://hg.mozilla.org/releases/mozilla-release/rev/25a6fa67de8d

Comment 25

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 62.0.2 (BuildId:20180920131237) on Windows 10 64bit, macOS 10.9.5 and Ubuntu 16.04 64bit.