Open Bug 1391298 Opened 7 years ago Updated 2 years ago

[meta] More sessionstore overhead when running speedometer

Categories

(Firefox :: Session Restore, defect, P2)

defect

Tracking

()

People

(Reporter: ehsan.akhgari, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: meta, perf)

This is very similar to bug 1373672, this time the function in question is ScrollPositionListener. The line number points to <https://searchfox.org/mozilla-central/rev/5ce320a16f6998dc2b36e30458131d029d107077/browser/components/sessionstore/content/content-sessionStore.js#495> for me, not sure which function observe is calling but this looks like a call coming from StateChangeNotifier. Please see this profile: https://perfht.ml/2vGXjL3 Again, the costly part is touching a content object which causes it to be Xray-wrapped.
Whiteboard: [qf] → [qf:p2]
Bao, are you interested to work on this bug by any chance? :-)
Flags: needinfo?(nnn_bikiu0707)
Sure :). I'm glad to work on this bug. The most important thing, I think, is to find out which observe function is showing in your profile. From there, we can find a way to solve this. I'll report back what I found.
Flags: needinfo?(nnn_bikiu0707)
:ehsan, can you take a look again at the profile [1]? It looks weird because there is a call to the |contains| function in FrameTree.jsm, which was removed in bug 1373672. Maybe this is from older version of Firefox? If so, can you reprofile again to see if the problem is still there? [1]: http://bit.ly/2wROqju
Flags: needinfo?(ehsan)
Huh I have no idea where the profile link in comment 0 comes from, my apologies, that looks like a totally different profile. :/ This is a trunk profile from Speedometer: http://bit.ly/2xzAhV7. I'm just going to morph this bug to cover the rest of the sessionstore overhead showing up in Speedometer. If needed we can split this into multiple dependencies, but I won't spend the time to look into the profile too deeply here. Here are a couple of big ticket items: * This timeout should perhaps be moved to the idle queue: https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/browser/components/sessionstore/content/content-sessionStore.js#806 * Do we need to process the DOMTitleChange event synchronously? https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/browser/components/sessionstore/content/content-sessionStore.js#369 I suspect not since the event itself is dispatched asynchronously: https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/dom/base/nsDocument.cpp#7289 If not, could this handling also be moved to the idle queue? There's probably more to dig into by more careful examination of the profile...
Flags: needinfo?(ehsan)
Summary: ScrollPositionListener is expensive when running speedometer → More sessionstore overhead when running speedometer
Priority: -- → P1
Priority: P1 → P2
Keywords: perf
Hey mikedeboer, I notice that the MessageQueue now uses the idle timer (yay!), but the DOMTitleChange stuff doesn't... do you know if that's planned?
Flags: needinfo?(mdeboer)
Whiteboard: [qf:p2] → [qf:meta]
Well, there's bug 1350569 that JanH mentioned a while back... solving that would mean even more perf wins than only moving the tab data collection to the idle timer and then we have bug 1395066 that's about doing just that IIRC, because it would involve doing that _and_ making sure we trigger a data collection way less often.
Flags: needinfo?(mdeboer)
Performance Impact: --- → ?
Keywords: meta
Whiteboard: [qf:meta]
Summary: More sessionstore overhead when running speedometer → [meta] More sessionstore overhead when running speedometer
Performance Impact: ? → ---
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.