Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] This add-on loads all tabs on start into memory #38

Open
TrashAccount3424 opened this issue Dec 31, 2023 · 13 comments
Open

[Bug] This add-on loads all tabs on start into memory #38

TrashAccount3424 opened this issue Dec 31, 2023 · 13 comments

Comments

@TrashAccount3424
Copy link

Since this add-on is meant to be used with TST it does not seem right to load every tab on startup to determine its color.
Most tabs, especially if there are many will be collapsed/hidden and do not need to be colored.

According to my tests with 1000 Tabs open, this add-on makes FF allocated several additional GBs (about 10GB) of RAM on start which slows down start enormously.
Tested with 1000 Tabs this add-on makes (re)starting FF take over 5 times as long as without the add-on enabled, all other things equal.

@wwp
Copy link

wwp commented Jan 8, 2024

Same here, when TST colored tabs is enabled, Firefox will eat a huge amount of memory at startup when TST initializes its tree in side pane. Most of the time it's got killed by kernel (out of memory). If TST colored tabs is disable, Firefox/TST load without delay or crash. That's an issue I met from time to time during the past few years, mostly when I had a big amount of tabs (even if disabled) in Firefox. I hadn't closed Firefox for a week or so, and now I cannot run at all with TST colored tabs enabled anymore.

That could be a duplicate of #33.

I also see similar crashes when disabling the add-on or when closing Firefox (not sure this is exactly the same bug).

firefox 115.6 esr (linux-64)
tst 3.9.19
tst colored tabs 0.7.1

@MurzNN
Copy link
Owner

MurzNN commented Jan 17, 2024

Thank you for reporting the details! Yes, the extension goes through all tabs and gets their URLs to colorize them properly, and seems Firefox do the full load of the tab to give me the url.

But I personally use this extension every day with around 50-200 tabs, and my browser is not restarted for weeks, without any memory usage problems.

So, do you have any ideas on how to resolve this memory usage issue? A mode to not colorize all existing tabs on startup looks not so good to me.

Maybe I can exclude colorizing collapsed tabs, but I doubt that it is possible to catch the "expand" event of a tab group, to colorize them on demand. If you have any recommendations for the implementation of this, please share.

Or maybe there is a way to get the tab url without triggering the tab load?

@TrashAccount3424
Copy link
Author

@MurzNN This is correct, the issue is only when the browser starts/restarts after the add-on has done the coloring the resources are freed but with a lot tabs on low spec hardware it may actually fail to finish or it can take several minutes.

A potential way to solve this would be to have the add-on do the initial coloring in small batches. It will take longer then but the browser should become responsive while its doing this. And it would not need to allocate all that RAM at the same time.

@MurzNN
Copy link
Owner

MurzNN commented Jan 17, 2024

I think batching by chunks will not resolve the problem, cuz Firefox will not instantly free up the memory from "activated" tabs in the previous batch chunk, it does the cleanup after a delay, maybe even for several minutes.

I'm trying to find an API to check if the tab is suspended (unloaded from memory) or not, but see nothing in the docs:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/Tab

And it's very strange that just accessing the tab.url activates the tab:

      for (let tab of tabs) {
        let host = new URL(tab.url);

Cuz Firefox should know the url without loading the full tab.

So, seems it's more like Firefox issue.

@MurzNN
Copy link
Owner

MurzNN commented Jan 17, 2024

By the way, I also use https://addons.mozilla.org/en-US/firefox/addon/auto-tab-discard/ that works well and frees up inactive tabs from the memory even on startup.

@wwp
Copy link

wwp commented Jan 17, 2024

Well, few details: I reproduced that crash at start-up on my 2 machines (a CentOS 7 and a Rocky Linux 9), both are equiped with 32GB of RAM, let's not talk of low spec hardware, but here in both cases I have a lot (hundreds) of tabs, and the amount of RAM is just enormous in the situation we're describing. Of course on low-spec machines it could be worse even with fewer tabs.
On both machines, Firefox was running for weeks, it is when I had to restart it that the crash has shown up - had to restart because an update of Firefox has forced me too on one machine, a system reboot on the other one.
I've already reproduced that same issue months ago (July?) and even before (one or 2 years ago). Each time I could workaround the situation by freeing memory (closing other apps) or starting Firefox w/ no TST add-on loaded, until few days ago where it was just not possible anymore: forced to stop the TST colored tabs add-on.
Unfortunately, I know nothing about the Firefox or TST internals, and cannot suggest anything here :-/.

@TrashAccount3424
Copy link
Author

I think batching by chunks will not resolve the problem, cuz Firefox will not instantly free up the memory from "activated" tabs in the previous batch chunk, it does the cleanup after a delay, maybe even for several minutes.

I'm trying to find an API to check if the tab is suspended (unloaded from memory) or not, but see nothing in the docs: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/Tab

And it's very strange that just accessing the tab.url activates the tab:

      for (let tab of tabs) {
        let host = new URL(tab.url);

Cuz Firefox should know the url without loading the full tab.

So, seems it's more like Firefox issue.

The free up of RAM is immediate after the add-on colored all tabs. I assume this is because they are all in scope until the very last is processed and only then the garbage collector will clean up all at once. Hence the idea of doing it in batches.

Also I dont think the tabs are ever activated, certainly the tabs do not load/render content or go online, it only loads cached stuff into RAM.

@MurzNN
Copy link
Owner

MurzNN commented Jan 17, 2024

So, all of the "touches" of tabs at startup are happen in this part of the code:

    browser.tabs.query({}).then(function(tabs){
      for (let tab of tabs) {
        let host = new URL(tab.url);
        host = host.hostname.toString();
        ColoredTabs.colorizeTab(tab.id, host);
      }
    }, onError);

It just gets the tab.url and tab.id, that's it! In all other code there are no calls to the Tabs API at all.
Seems the tab.url (or tab.id?) unsuspends the tab? That sounds like a bug in Firefox API.

So I have no idea about how to optimize this.

@TrashAccount3424
Copy link
Author

By the way, I also use https://addons.mozilla.org/en-US/firefox/addon/auto-tab-discard/ that works well and frees up inactive tabs from the memory even on startup.

I think by default FF has all tabs that are not pinned "unloaded" at startup, this add-on does not seem to change the coloring behavior.

@wwp
Copy link

wwp commented Jan 17, 2024

I use auto-tab-discard and I think it doesn't help at all WRT our problem here.

@TrashAccount3424
Copy link
Author

So, all of the "touches" of tabs at startup are happen in this part of the code:

    browser.tabs.query({}).then(function(tabs){
      for (let tab of tabs) {
        let host = new URL(tab.url);
        host = host.hostname.toString();
        ColoredTabs.colorizeTab(tab.id, host);
      }
    }, onError);

It just gets the tab.url and tab.id, that's it! In all other code there are no calls to the Tabs API at all. Seems the tab.url (or tab.id?) unsuspends the tab? That sounds like a bug in Firefox API.

So I have no idea about how to optimize this.

I think the code that is responsible for the RAM is here:

browser.runtime.sendMessage(TST_ID, {
type: 'remove-tab-state',
tabs: [tabId],
state: ColoredTabs.state.tabsClass[tabId],
});

This is executed for each tab nearly at the same time causing parallel execution on all tabs at the same time.

@MurzNN
Copy link
Owner

MurzNN commented Jan 24, 2024

I started to face today memory issues with TST too, and even with TST Colored Tabs disabled, posted more info here piroor/treestyletab#3415
So, please try to disable the "Optimize tree restoration with cache" feature and check if this helps.

@TrashAccount3424
Copy link
Author

I started to face today memory issues with TST too, and even with TST Colored Tabs disabled, posted more info here piroor/treestyletab#3415 So, please try to disable the "Optimize tree restoration with cache" feature and check if this helps.

Tested this setting and it has no effect on the bug described here.

While testing I figure out, the simplest way to reproduce this bug is to simply to click on the "Save and apply settings" button.

TST Colored Tabs > Options > Save and apply settings

This should re-color all tabs and run the same code as on startup of firefox.
The moment I press that button FF becomes unusable, content in open widows freeze and RAM starts to fill up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants