1168428 - [meta] Sync is janky

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P3)

Description

[Luke Wagner [:luke]]

Today, when I switched to a FF profile that I hadn't used in a while (so that there was a lot to sync in my FxA), I noticed the browser chrome was really laggy (variable second delays between mouse action and UI response in browser chrome).  I restarted the browser (which had just finished updating to the latest nightly build), and after half a minute or so, the UI lagginess returned.  I noticed that the Sync animation was spinning in the FF menu indicating that a Sync operation was going on.  After a minute or so, the Sync operation completed and the UI was immediately responsive again.  I'm afraid I didn't have time to capture a profile while this was happening.

Comment 1

[Luke Wagner [:luke]]

I forgot to add, this just happened on Win7, but it also happened for me on OSX recently (same situation: using profile linked to big FxA for the first time in a month) and the browser was almost unusable for a minute.

Comment 2

[Bill Maggs (bmaggs)]

Needs investigation and should be assigned as high priority.

Comment 3

[Chris Karlof [:ckarlof]]

This could be users' second device experience as well.

Comment 4

[Edwin Wong [:edwong]]

:pdehaan any chance you can try to repro this? We'd love more data on how many people experience this.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

A quick profile shows the lowest hanging fruit is the bookmarks engine, and specifically _buildGUIDMap(). Best I can tell, this is all using the old sync bookmarks API, which I believe does main thread IO. We probably want to convert this engine to using async IO (but I also wonder if we should consider looking at the OSX bookmarks engine rnewman recently did, and just rewrite the entire engine!?

Comment 6

[Ryan Kelly [:rfkelly]]

> A quick profile shows the lowest hanging fruit is the bookmarks engine, and specifically _buildGUIDMap()

I've heard this _buildGUIDMap() thing mentioned before, so I wonder if there's existing bugzilla literature on plans for its removal.  Couldn't find anything using my primitive bz search skills though...

Comment 7

[Chris Karlof [:ckarlof]]

Related is Bug 1008592

Comment 8

[Mark Hammond [:markh] [:mhammond]]

I'm going to treat this as a meta bug for all janky sync-related things. I'm unassigning myself from this one, but I'll take most (all?) of the dependencies.

Comment 9

[Chris Karlof [:ckarlof]]

Clearing the priority from all our meta bugs.

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Hacks-to-try-and-identify-jank-in-Sync.patch

Kit reports this has come back recently. I just rebased the earlier hacks I had in place to measure jank (but haven't verified it still works) and I'm attaching it here incase someone else wants to have a look - needinfo myself to play with this some more later.

Comment 11

[Lina Butler [:lina]]

Mark was looking at jank in _guidMap, and fixed the issue in bug 1303831.

Comment 12

[Mark Hammond [:markh] [:mhammond]]

Attachment: hacks-to-idenfity-jank-in-sync.patch

Updated to current master.

Comment 13

[Mark Hammond [:markh] [:mhammond]]

All dependencies are closed \o/