Closed Bug 1869928 Opened 3 months ago Closed 2 months ago

syncThrobberAnimations takes a lot of time at startup when there are many tabs

Categories

(Firefox :: Tabbed Browser, defect, P3)

defect

Tracking

()

RESOLVED FIXED
123 Branch
Tracking Status
firefox123 --- fixed

People

(Reporter: sfoster, Assigned: sfoster)

References

Details

Attachments

(2 files)

I'm attaching a saved session with 1000 tabs. To use:

  • Check the "Open previous windows and tabs" checkbox in about:preferences and quit the browser
  • Copy in the attached sessionstore-1000-tabs.jsonlz4 file into the profile directory and rename it sessionstore.jsonlz4 (don't use a valuable profile, or back up your sessionstore-backups folder if you do.)
  • Start the browser. It will probably hang for a few seconds while it reads the session file and creates all those tabs

I captured this profile starting up with this. It shows several seconds spent in syncThrobberAnimations. Which is odd because only the selected tab (and any pinned tabs) should be actively loading. The function tries to get the animations from each and every tab and then loops over them to sync all their startTimes. I think we filter for only the tabs that newTab.hasAttribute("busy").

Assignee: nobody → sfoster
Status: NEW → ASSIGNED

I'm curious, sfoster - did you notice any difference in the time to restore from that large session with this patch applied?

Flags: needinfo?(sfoster)

Nevermind, I see that this was answered in https://phabricator.services.mozilla.com/D196537#6520090.

Flags: needinfo?(sfoster)

The severity field is not set for this bug.
:dao, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(dao+bmo)
Severity: -- → S3
Flags: needinfo?(dao+bmo)
Priority: -- → P3
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5b8099aad23d
Only sync throbber animations for tabs that are animating. r=mconley,tabbrowser-reviewers

Ah, thanks I'll take a look and update the patch (and see if mach try auto would have caught this before I pushed.)

Flags: needinfo?(sfoster)
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd963f1a3745
Only sync throbber animations for tabs that are animating. r=mconley,tabbrowser-reviewers

To get some measurements of the before and after I used the STR from comment #0, and ran and then quit the browser to capture a startup profile:

`MOZ_PROFILER_STARTUP=1 MOZ_PROFILER_STARTUP_INTERVAL=10 MOZ_PROFILER_SHUTDOWN=/path/to/output/profile.json MOZ_PROFILER_STARTUP_ENTRIES=20000000 ./mach run`

And then loaded up the output file at https://profiler.firefox.com/, and filtered down the markers in the marker table view of the parent process with sessionrestore-.

We create a perf marker at sessionstore-windows-restored which is when the session data has been read and the windows created, but before any tabs are created. The sessionstore-one-or-no-tab-restored marker indicates the moment during startup when all the tabs have been restored. That is, the tab elements are created (most remain inactive and don't actually load anything until selected.) The time between the two moments is where this patch has an impact:

session / patch sessionstore-windows-restored (s) sessionstore-one-or-no-tab-restored (s) time to restore tabs (s)
1000 tabs (m-c 4.569 11.503 6.934
1000 tabs (m-c 4.723 12.864 8.141
1000 tabs (m-c 4.641 11.434 6.793
1000 tabs (m-c 4.508 14.403 9.895
1000 tabs (m-c 4.528 11.427 6.899
1000 tabs (patched) 5.531 8.949 3.418
1000 tabs (patched) 4.517 7.985 3.468
1000 tabs (patched) 4.516 7.902 3.386
1000 tabs (patched) 4.537 7.763 3.226
1000 tabs (patched) 4.601 7.996 3.395
500 tabs (m-c) 3.995 7.61 3.615
500 tabs (m-c) 3.329 6.63 3.301
500 tabs (m-c) 3.58 7.26 3.68
500 tabs (patched) 4.038 6.726 2.688
500 tabs (patched) 3.726 5.875 2.149
500 tabs (patched) 3.343 5.93 2.587
100 tabs (m-c) 2.689 6.836 4.147
100 tabs (m-c) 2.476 4.677 2.201
100 tabs (m-c) 2.551 4.781 2.23
100 tabs (m-c) 2.663 4.958 2.295
100 tabs (patched) 2.426 4.574 2.148
100 tabs (patched) 2.386 4.265 1.879
100 tabs (patched) 2.378 4.514 2.136

Here's that distilled to just the time we spent restoring the tabs with and without the patch:

session / patch median time to restore tabs (s)
100 tabs (m-c) 2.2625
100 tabs (patched) 2.136
500 tabs (m-c) 3.615
500 tabs (patched) 2.587
1000 tabs (m-c) 6.934
1000 tabs (patched) 3.395
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 123 Branch
Blocks: 1878450
You need to log in before you can comment on or make changes to this bug.