1460951 - Sync spends a lot of time in Garbage Collection

Status: RESOLVED DUPLICATE of bug 1536170

(Firefox :: Sync, defect, P2)

Description

[hugbear]

Upon signing in to Sync the CPU usage dramatically increases (up to 60%-90%). It never goes back down to normal, even after idling for 8-10 hours with no open page.

Disconnecting from account does NOT bring CPU usage down.

The only way for Firefox to behave normally is to restart it after disconnecting (therefore NO SYNC).

1. Firefox should cap CPU allocation for syncing.
2. Firefox should actually finish syncing at some point, thus reducing its CPU usage.

Comment 1

[Lina Butler (ex-Mozilla)]

Thanks for uploading that profile! Can I ask you how many bookmarks you have in your profile?

I see `addRowToChangeRecords` appear multiple times, which is interesting because it's a fairly simple function. In a different bug (bug 1461136), though, I see a lot of lines like this in the logs:

> 1526099602374	Sync.Engine.Bookmarks.BookmarkSyncUtils	ERROR	addRowToChangeRecords: Invalid modified date for BhxK_xDC7lpg: 0

Do you see anything like that if you open about:sync-log and inspect one of the error logs? We fixed the underlying issue in bug 1455906, run as part of maintenance, but that's only in Beta 61. I also wonder if something else is going on.

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

Sadly, this looks like we're spending a ridiculous amount of time in garbage collection.

I suspect this is another example of jankYeilder causing jank by allocating many many promises.

It's not clear what the best path forward for avoiding this is, however. (I have some thoughts on how we could, but it's not clear how to do it in a way with reasonable ergonomics that isn't likely to get reintroduced).

Clearing the ni for hugbear since the profile looks like it's much more related to GC issues than bookmarks issues.

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

I know what to do here but won't have time for a while (replace with functions that contain the loop inside of them which inlines the check to avoid the unnecessary promise generation. There are a couple ways worth noting that we can optimize this too, if someone other than me does this please talk to me first).

Anyway, MyQOnly now bugs me about needinfos, so I'm clearing this one.

Comment 4

[Mark Hammond [:markh] [:mhammond] [PTO until Sept 7]]

We suspect most of this has been fixed by bug 1536170