More sessionstore overhead when running speedometer

NEW
Unassigned

Status

()

Firefox
Session Restore
P2
normal
9 months ago
4 months ago

People

(Reporter: Ehsan, Unassigned)

Tracking

(Blocks: 2 bugs, {perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf:meta])

(Reporter)

Description

9 months ago
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.
Blocks: 1330635
(Reporter)

Updated

9 months ago
Blocks: 1339557
Whiteboard: [qf] → [qf:p2]
(Reporter)

Comment 1

9 months ago
Bao, are you interested to work on this bug by any chance?  :-)
Flags: needinfo?(nnn_bikiu0707)

Comment 2

9 months ago
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)

Comment 3

9 months ago
: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)
(Reporter)

Comment 4

9 months ago
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
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)
You need to log in before you can comment on or make changes to this bug.