[Bug] Slow sidebar performance in 3.9.20 when "Persist tree cache" is unchecked #3434
Comments
|
The change you pointed means TST switched the backend of the data store from Serializing/deserializing helper is defined at: On the other hand, old backend Thus I guess that this difference possibly causes slowing down on your environment. A performance profile with the debug tool (Browser Toolbox => Performance => "Firefox" profile => "WebExtensions" process) looks to be needed for more inspection. |
|
And, of course TST 3.9.20 contains more other changes, so slowing down possibly happens due to such other changes. Collecting performance profile is a required step anyway. |
|
Another problem when the tree cache is enabled is large memory consumption (once I noticed the WebExtension process having about 10G RSS). Also collecting the memory report on Top of the memory report for the extension process after enabling the cache and opening and closing a couple of empty tabs: Apparently the number of objects like |
|
There is definitely a leak, I ended up here because in my case the strings consume 25gb and ff is incredibly slow... |
|
Hmm... activating "Persist tree cache" is the workaround for now, if it is a problem of |
|
Yes, I have checked and unchecked that option a few times and that deflates the ram usage to 1gb, which is fine, and keeping it checked restores the old, fast, firefox, that does not eat my ram. |
|
Same here. Disabling "Optimize tree restoration with cache" seems like solving issue. And what is more interesting - I don't see any visual performance benefit from that option at all (even with my 1086 tabs tree!). Is it still relevant? p.s. I didn't try to use "Persist tree cache" |
From limited testing, disabling "Optimize tree restoration with cache" works equally well as a workaround, if the cache is not needed |
|
Here is a profile of opening a new (searchfox) tab. My process was:
Note that the stack frames mentioning Note that in addition to the time spent in the WebExtensions process deserializing the data, there is significant time spent in the parent process main thread serializing the data which will cause problems as well. It's worth noting that WebExtensions are able to use storage APIs like IndexedDB directly and those don't involve the main thread of the parent process. But any performance issues related to large object (graphs) being serialized/deserialized will potentially remain. Although using IDB directly does potentially allow for potentially allow for more granular storage since something that might currently be a single map from N tab identifiers to N tab payload could instead be stored as N separate records which could be retrieved at startup using a cursor over the lexicographic range that could contain tabs. (We/the Gecko DOM storage team have put a lot of work into optimizing cursor prefetching, so this can be very performant while also helping avoid blocking the WebExtensions main thread like if getAll() was used, since work can be sliced up. It also avoids concerns about record sizes, somewhat.) |
|
I've created three similar branches.
I hope they all should work better than TST 3.9.20. 1 should be better than 2, but 1 is possibly slower than 2. 3 should better than 2 also but I have a concern about increased disk I/O. (See also details: #3434 (comment) ) Could you try these versions, with enabled cache and disabled permanent cache? Here is the instruction to try development builds: https://github.com/piroor/treestyletab#development-builds |
|
|
|
Thank you so much! (And thank you for creating and maintaining this incredible extension. I love TreeStyleTab so much! It is a must-have, must-use extension, and I am lost whenever I try and use a browser without it!) Initial testing with all 3 variations without changing any settings (so To anyone else testing the above builds, something that was not immediately obvious to me is that when I clicked on the "treestyletab-we.xpi" artifacts, the download of "treestyletab-we.xpi.zip" that resulted was in fact a zip file containing the XPI file, not just a mis-named XPI file. You do need to unzip that zip file to get to the XPI inside. Otherwise you will get an error about an invalid manifest. |
|
The "non-persistent cache" mode was originally introduced as a workaround for the issue #3388 which is about slowing down from bloated session file - increased disk I/O on classical HDD (non-SSD) would triggers slowing down of the Firefox process, when the cache became huge from a tree cache for a large number of tabs. Firefox's IndexedDB implementation is based on SQLite, and stored caches are finally wrote to an SQLite file like I have two Windows 11 PCs and both they use SSD as their main storage, so I cannot feel slowing down from increased disk I/O seriously. I need more testing reports of three development versions, from people who use large number of tabs with non-SSD storage. |
|
For #3388 I think https://bugzilla.mozilla.org/show_bug.cgi?id=1849393 is important context. It's currently a known problem that the session store (which is distinct from SessionStorage, but which is responsible for persisting SessionStorage data) performs a single JSON serialization of a very large object on the main thread of the parent process which can block the main thread for 0.5 seconds easily for users with a lot of tabs/windows. (And I believe in that bug, non de-duplicated favicons are called out as a problem, although I think that's for the session store's own use, not TST.) To be very clear, the I/O happens on another thread, it's the call that amounts to JSON.stringify() that's the problem. And because the I/O is a sequential write, it actually should be fairly efficient on spinning hard disks. As I note above, since SessionStorage will also end up stored in the session store file, it's likely that IndexedDB is the best choice, especially since it avoids the main thread of the parent process entirely. As you say, the IDB writes should generally be smaller since our implementation uses SQLite and changes will be limited to modified btree pages and their parental tree structure. The trade-off is that although we will perform sequential writes to the SQLite Write Ahead Log (WAL), when we checkpoint the WAL, we will need to perform random access writes as the pages are transferred into the main database file. I think the main concern would likely be if values are set in the cache frequently, it could make sense to attempt to accumulate changes in memory and then write them to IDB in a single transaction after a timeout or when you find you need to read one of the values, so that your reads can see the writes as part of the same transaction. This would reduce I/O overall. It's probably also worth noting that our IndexedDB implementation does a clever thing with Blob/File storage. Any Blob/File that is stored in the database will be file-backed once it is read back out, and if you re-store that Blob, we will simply increment the reference count for the Blob in the database rather than storing the value. We also have an optimization if you store a Blob/File multiple times. But note this is based on object identity, not on content de-duplication. So if you do |
|
I should probably also mention that in relation to #3387 Firefox does now support storing IndexedDB data in Private Browsing, which it sounds like was impacting people using Firefox in permanent private browsing mode which unfortunately causes loaded WebExtensions to have an mPrivateBrowsingId=1 origin attribute. While the WebExtensions storage APIs go out of their way to not discard the data, QuotaManager's PrivateBrowsing implementation fundamentally must discard the data when the browser restarts. (The data is stored in an encrypted database where the keying material is never saved to disk.) I think in the medium-to-long term the WebExtensions team would like to try and change the Private Browsing mode behavior of WebExtensions under permanent private browsing mode, as I don't think that was ever an intentional action[1], just a side effect of how private browsing windows work (they are created with mPrivateBrowsingId=1 and that flows to all of the windows they contain) and how permanent private browsing mode meant that all windows were private browsing windows. 1: There are intentional behaviors around not exposing the existence of private browsing windows/tabs to WebExtensions that haven't been granted that ability, but that's separate from putting the WebExtension itself into private browsing mode. |
|
@asutherland Thank you for detailed explanation! Large cache data for sidebar contents (
|
|
Sorry, linked revisions have bugs: caches are not used for sidebar. I've updated links to development versions: #3434 (comment) |
|
On my environment |
|
I've tried four branches and compare their RAM usage with 525 tabs.
( Reload TST => minimize memory usage => add blank tab => close blank tab => measure )
Please note this report ignores the overhead of serializing and deserializing intentionally. There may be different winners about less spiked CPU and RAM usage. Edit: I've updated the table with IndexedDB + blob. |
|
Note that Additional allocations in the IDB case I think would be something like an increase in the number of IPC buffers retained by the IPC subsystem and not something specifically retained by IDB. The SQLite connection itself should target a page cache of only 2 MiB, for example. This can potentially be reduce by moving to wrap and store your string in a Blob rather than storing the string directly as a value in the object. This would reduce the serialized IPC message size. The data in the blob would instead be sent via a different IPC mechanism to stream its contents to the parent process and to disk (using a more bounded circular buffer mechanism). The downside to this is that when you get the result values back from the database, you will have a Blob which you will still need to issue an async request to read. Although it does open the possibility of being able to create a blob URL from the blob and to provide that directly to an iframe as its source URL, which could in turn allow for off-the-main-thread HTML parsing (potentially). But that gets potentially quite complicated unless you have cached the entire HTML page. One can of course build composite blobs like |
|
I have unchecked the "Optimize tree restoration with cache" option to avoid the performance degredation I've experienced in the past few days, e.g. very slow and more than 10 GB RAM use by the extension. Is there a disadvantage to disabling the cache, apart from increased startup time? What is the function of the in-memory non-persistent cache, if it's not tree restoration? |
|
Another victim of this patch - i just had to close FF due to slow opening/switching/closing tabs, and about 40GB of ram usage due to TST. It started like 5-15 days ago, probably same thing. |
|
Hi, I am also suffering due to this issue. If I do not disable Tree Style Tabs or enable “Persist tree cache” work, the browser becomes almost unusable for typical browsing due to this: every page load is delayed by several seconds, and while the Firefox tab bar is responsive, the TST tab bar is very, very unresponsive. Performance profiling of a single page load showed over 40,000ms (yes, 40s) spent in JavaScript inside the WebExtension. I also noticed the WebExtensions process taking up something like 4GB when I used macOS's Activity Monitor. Disabling Tree Style Tabs or enabling “Persist tree cache” do seem to work around it at least. If the experience is as bad for others as it is for me, I wonder if it is an option to temporarily revert the original change. |
Unfortunately what I am fearful of is what had happened to me many years ago, whereas, I believe in recent years for whatever setting's reason I have no run into this issue. I.e; very rarely, if FF was to crash or if TST malfunctioned in anyway on its startup, there was the potential for all tabs to be lost. I had found myself using addons like Copy All Tab URLs occasionally just as an insurance method at times. I've been experiencing these RAM issues as many here have for months maybe even almost all of last year. It's had me attempting to isolate all kinds of things in about:config too... As many have spoken of their resolution by unchecking "Optimize tree restoration with cache", I have done so too and it seems to totally rectify the issue(This was before updating to 3.9.22, from 3.9.20), I have an alternate profile which is on 3.9.20 for which I attempt to fiddle with "Optimize tree restoration with cache" and its sub persist option for additional heuristics to see if the problem persists there. As with -#3415 (comment) I wonder if I can revert back to using the option of "Optimize tree restoration with cache"(of which I was not using the persist sub option before, but now I wonder if I should in 3.9.22), since I worry without these options I risk the potential hazard of total tab loss. -Running with "Optimize tree restoration with cache" checked in 3.9.22 for a while to see if I run into the RAM issues. I will actually turn on browser.cache.memory.enable for a while now too as I thought that may have been an issue, after I was fiddling with it & its other related about:config memory cache settings :| |
|
Related topic: |
I am sure I speak for most users when I say we look forward to your evolution of TST @piroor, your inspirational seeking of unique technologies is mind-blowing and your endless consideration & open-mindedness regarding your addon is greatly appreciated. Thanks so much, looking forward to the virtual scrolling architecture in time to come. |
|
This issue has been labeled as "stale" due to no response by the reporter within 1 month (and 7 days after last commented by someone). And it will be closed automatically 14 days later if not responded. |
|
The experimental branch |
|
This issue has been labeled as "stale" due to no response by the reporter within 1 month (and 7 days after last commented by someone). And it will be closed automatically 14 days later if not responded. |
|
Do the underlying issues also produce a constant CPU load in the plugin (according to |
|
This issue has been closed due to no response within 14 days after labeled as "stale", 7 days after last reopened, and 7 days after last commented. |
In the 3.9.20 patch notes, I see
This behavior change seems to have a noticeable impact on TST performance in certain scenarios. Poor performance includes things such as:
Performance degradation is only noticeable when a large number of tabs are open and also seems to require that "Optimize tree restoration with cache" is checked
Steps to reproduce
Environment
The text was updated successfully, but these errors were encountered: