1863783 - Switching tabs is slow because of Firefox View

Status: RESOLVED FIXED

(Firefox :: Firefox View, defect, P1)

Description

[Jeff Muizelaar [:jrmuizel]]

Profile is here: https://share.firefox.dev/49vQtJ9

I was switching back and forth between tabs. I'm seeing a bunch of time in chrome://browser/content/firefoxview/opentabs.mjs:render()

It seems like Firefox View is adding about 300ms to every tab switch.

Comment 1

[Sarah Clements [:sclements]]

Are you seeing this while in Firefox View and switching between tabs or not within View? And with a large amount of tabs?

Comment 2

[Jeff Muizelaar [:jrmuizel]]

This is happening while not using Firefox View. I have 2066 tabs.

Comment 3

[Sarah Clements [:sclements]]

I suspect the problem is that we keep event listeners and observers active for opentabs.mjs even if Firefox View isn't visible. It should be doing what other sections do and removing all of those when a user navigates away from View. There were helpers added in ViewPage for this reason.

Sam I know you're knee deep into the Open Tabs refactor right now. Does this get addressed in one of your patchs? Or do you have a WIP that I could piggy back on? This should be a simple change so we should try to get it into 121 if possible.

Comment 4

[Sam Foster [:sfoster] (he/him)]

(In reply to Sarah Clements [:sclements] from comment #3)

Sam I know you're knee deep into the Open Tabs refactor right now. Does this get addressed in one of your patchs? Or do you have a WIP that I could piggy back on? This should be a simple change so we should try to get it into 121 if possible.

No this isn't directly addressed in my other patches. I think I can shuffle my queue and move this up to the front though. We do listen for visibility changes on the fxview document, and we already track which of the category pages is selected. I think we can extend and combine these to shut down all the observers and event listeners when a given page is not visible for whatever reason, and start the listening and observing for updates only when it is. That should make fxview inert when not selected and visible to the user, which should address this bug.

Comment 5

[Sarah Clements [:sclements]]

Thanks for picking this up Sam.

Comment 6

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1863783 - Ensure fxview page content only listens/observes and renders updates when visible. r?#fxview-reviewers

• Add a .page class to the top-level ViewPage instances
• Rename viewTabVisibleCallback and viewTabHiddenCallback to view* and call each when selectedness or visiblity changes
• Ensure active view/pages are always properly initialized during page load and category switching
• Add a test to verify no mutations happen when open tabs change while firefox view is inactive

Comment 7

[Julien Wajsberg [:julienw]]

I see this issue is in good hands, but in case this is useful here is a profile from my computer: https://share.firefox.dev/3SLMBh8. It's zoomed in on a specific switch but you can zoom out to see more if necessary.

Hope this helps

Comment 8

[Julien Wajsberg [:julienw]]

I'm surprised especially by the amount of GC. (this is also in Jeff's profile). Do you have an idea why this happens?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7015c326ee31
Ensure fxview page content only listens/observes and renders updates when visible. r=fxview-reviewers,sclements,jsudiaman

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/7015c326ee31

Comment 11

[Sarah Clements [:sclements]]

We'd like to consider this for beta uplift next week, but we want to let it sit in Nightly for a week or so and have QA test this early if possible.

Comment 12

[Sam Foster [:sfoster] (he/him)]

:jrmuizel, are you able to verify this fixes the issue for you? The patch addresses the case where tabs are being selected/opened/closed/moved while the open tabs or recent browsing list is not visible. Ie. either you don't have firefox view open at all, or are on a different view/panel like recently-closed tabs.

Comment 13

[Catalin Sasca, Desktop Test Engineering [:csasca]]

We tried verifying the fix but unfortunately it is too much to handle for the systems to open ~2k tabs at once (Ubuntu will crash completely because it is out of memory). We tried by opening a large amount of imported history and by using the profile shared by Sam in Bug 1859829.

Comment 14

[Catalin Sasca, Desktop Test Engineering [:csasca]]

On another note, no performance issues or strange behavior was seen while testing typical scenarios on the fixed builds.

Comment 15

[Sam Foster [:sfoster] (he/him)]

(In reply to Catalin Sasca, Desktop QA [:csasca] from comment #13)

We tried verifying the fix but unfortunately it is too much to handle for the systems to open ~2k tabs at once (Ubuntu will crash completely because it is out of memory). We tried by opening a large amount of imported history and by using the profile shared by Sam in Bug 1859829.

Can you use the session file and procedure I attached to https://bugzilla.mozilla.org/show_bug.cgi?id=1858478#c6?
This will open 1000 tabs. But we only load the tab content when the tab is selected, so this shouldn't take up too much memory at startup. If that's still too much let me know and I can attach another session file.

Comment 16

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Sure thing. Used the session file and procedure attached to https://bugzilla.mozilla.org/show_bug.cgi?id=1858478#c6 and I couldn't reproduce any slowdowness/lags while opening random tabs on the affected build (2023-11-08). On the other hand the fixed build (2023-12-03) seemed a bit more snappier on macOS 13.6.1.

Comment 17

[Jeff Muizelaar [:jrmuizel]]

Yes, this seems to solve the issue I was seeing. I don't see any firefox-view time during tab switching anymore.

Comment 18

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 9363835 [details]
Bug 1863783 - Ensure fxview page content only listens/observes and renders updates when visible. r?#fxview-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Users with slow machines and/or large numbers of open tabs may find browser performance slower after using firefox view
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low+ risk maybe? The changes are limited to firefox view. But the patch changes how and when all firefox view content is updated, making it so all updates (via observers and event listeners) are paused when the given section in firefox view (or the tab itself) is hidden, and resumed when it becomes visible.
• String changes made/needed: None
• Is Android affected?: No

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9363835 [details]
Bug 1863783 - Ensure fxview page content only listens/observes and renders updates when visible. r?#fxview-reviewers

Approved for 121.0b8.

Comment 20

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/cfb4c5947636