SessionWriter freezes Firefox when using Simple Tab Groups and Tree-style Tabs extensions
Categories
(Firefox :: Session Restore, defect, P3)
Tracking
()
People
(Reporter: nbp, Unassigned)
References
(Depends on 1 open bug)
Details
Since a week, Firefox is freezing, taking over the CPU and not responding to any input for seconds/minutes, both on Windows and Linux.
Using the profiler, once I noticed a spike starting on htop … I managed to capture the following profile:
https://share.firefox.dev/3OhUnwc
This suggests that this might be an issue, either:
- In the SessionWriter which attempts to write too much things.
- In the the DOM / JS which are failing to optimize very large string allocations by making repeated calls to Append.
- In the code responsible for escaping character within SpiderMonkey.
This bug might be a duplicate of Bug 1849393.
|
||
Comment 1•10 months ago
|
||
I noticed this had gotten much worse for me a couple of weeks ago. Bug 1849393 has affected me for a long time, but it was fairly hard to notice. Now my browser is constantly lagging in all operations, and when I take a profile, it's the sessionstore writing.
|
|
||
Comment 2•10 months ago
|
||
In my profile there is also a lot of time spent in the WebExtensions process. This might be related to having lots of tabs and using Tree Style Tabs? The extension process spends a bunch of time in both JSON serialization and structured clone handling.
|
|
||
Comment 3•10 months ago
|
||
Disabling Tree Style Tabs fixes my performance issue.
|
||
Comment 4•10 months ago
|
||
Could we do the equivalent of mozregression with older treestyletabs versions to see what changed that is then triggering session restore to take much longer?
This looks potentially related to https://github.com/piroor/treestyletab/issues/3434
If this is the same thing as https://bugzilla.mozilla.org/show_bug.cgi?id=1827433 it's back and getting on my nerves. I hate how it's fine for months and then Firefox updates something and breaks it all over again.
|
|
||
Comment 6•9 months ago
|
||
(In reply to Maswartz from comment #5)
If this is the same thing as https://bugzilla.mozilla.org/show_bug.cgi?id=1827433 it's back and getting on my nerves. I hate how it's fine for months and then Firefox updates something and breaks it all over again.
Are you using treestyle tabs? Is it the latest version? Does the problem stop if you disable it temporarily?
If you can figure out what exactly broke this (by running https://mozilla.github.io/mozregression/ ) that would also be helpful.
|
Reporter | |
Comment 7•9 months ago
|
||
I am using Simple Tab Groups, which is probably worst in terms of meta-data per tabs.
I will try to bisect Firefox over multiple days.
I have no idea what treestyle tabs even are and I'm using the latest firefox.
|
||
Comment 9•9 months ago
|
||
:robwu, do you have other reports or other details here on how these and other extensions use the session api and what approaches we might look at to get a better experience here?
|
||
Comment 10•9 months ago
|
||
The relevant code responsible for enabling extensions to bloat the sessionstore file is at:
- https://searchfox.org/mozilla-central/rev/26dd94a024f21971e539a8650c2f177323c98fe2/browser/components/extensions/parent/ext-sessions.js#242-294
- https://searchfox.org/mozilla-central/rev/26dd94a024f21971e539a8650c2f177323c98fe2/browser/components/extensions/parent/ext-sessions.js#77-87
The SessionStore logic that is responsible for associating the custom values with the internals (that are stored) is at: https://searchfox.org/mozilla-central/rev/26dd94a024f21971e539a8650c2f177323c98fe2/browser/components/sessionstore/SessionStore.sys.mjs#4169-4240
I don't see anything stopping extensions from adding large amounts of data to sessionstore entries, and this has been the case since the feature was introduced in bug 1322060.
Since this value is not critical to session store, it is technically feasible to use a separate storage mechanism to keep track of this extension-specific data: e.g. instead of storing the data in the main sessionstore file, store a (new) database entry identifier, and use that identifier to look up the actual value when extensions are interested in it. Plus a way to erase stale entries from the database, and migration code.
The question is whether this would be desired by users - is extension-specific state (added by tab managers) part of the sessionstore file? Do users expect extension-defined session state to be preserved when they make a backup of the sessionstore files?
|
||
Comment 11•9 months ago
|
||
I (Tree Style Tab developer) agree that the sessions.set/getXXX APIs are not designed to store huge data frequently. I used sessions.setXXX APIs too carelessly because it is very easy way to store some data to restorable tabs and windows, so now I'm trying to update TST to reduce usage of sessions storage. The bloated data was due to storing caches, and the latest released version TST 3.9.22 now stores caches into its own IndexedDB datastore and only short identifier (still required to track tabs and windows across sessions) and some metadata are left. I've wrote codes to clear old caches from the sessions storage, but some caches may be left for session data for restorable tabs and windows - sessions.set/removeXXX APIs require an ID of living tab or window so it is impossible to remove obsolete data from session data for unexisting tab or window.
Session-related addon data bundled to the session file is still useful from some reasons, like easy to backup. So I hope that the present mechanism is basically kept, but introducing quota limit is acceptable. Addon developers like me needs something guideline how to store huge data across browser sessions at API references ( https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/sessions/setTabValue and https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/sessions/setWindowValue ), like: "You should not store huge data via sessions.setTabValue or sessions.setWindowValue from the quota limit, instead you should assign generated identifier strings for each window and just storing them via sessions.setTabValue/setWindowValue, then you would use it as keys to write and read huge data with any arbitrary storage like IndexedDB."
Description
•