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

gigabytes of memory used for icons on extension load #471

Closed
hyperfekt opened this issue Oct 27, 2019 · 22 comments
Closed

gigabytes of memory used for icons on extension load #471

hyperfekt opened this issue Oct 27, 2019 · 22 comments

Comments

@hyperfekt
Copy link

Short description

With 17 sessions a ~2000 tabs, loading the extension immediately loads a large amount of icons into memory, leading to permanent multi-gigabyte memory use.

Steps to reproduce

  1. Start browser with clean profile
  2. Install Tab Session Manager
  3. Create very large sessions.
  4. Disable Tab Session Manager.
  5. Restart browser.
  6. Enable Tab Session Manager.

Expected result

Firefox memory use does not change materially as long as no action is performed using Tab Session Manager.

Actual result

Memory usage of the WebExtension host process rises to several gigabytes immediately,
about:memory shows that to be due to tens to hundreds of thousands of strings, all beginning with data:image/x-icon;base64.

Platform information

  • Platform (OS): Linux
  • Version of browser: 69.0.2
  • Version of Tab Session Manager: 5.1.0

Additional context

@sienori
Copy link
Owner

sienori commented Nov 4, 2019

I plan to implement a feature that replaces the too long base64 favicon URL with google favicon api.
Thanks report!

@klorinczi
Copy link

Waiting to lower memory usage.
Even 90 windows with total of 400-500 tabs is unusable.

@1RonRock
Copy link

1RonRock commented Dec 14, 2019 via email

@pecastro
Copy link

pecastro commented Mar 11, 2020

Same thing here ...
And you don't need that many windows open.
All you need is a single tab, once you activate the addon it starts eating memory away ...
#337 (comment)

@deliciouslytyped
Copy link

deliciouslytyped commented Nov 22, 2020

I plan to implement a feature that replaces the too long base64 favicon URL with google favicon api.

Please allow me to disable this feature (and perhaps favicons completely) - I don't want to be submitting all my tab urls to the network, or to google. There is no reason the addon should need network access to manage my session.

@sienori sienori added this to the Replace favicon URL milestone Dec 9, 2020
@sienori
Copy link
Owner

sienori commented May 2, 2021

I've implemented the ability to compress the favicon url when saving the session! 1c6ac8a

In Firefox, favicon URL are saved as a long string called data URL.
This update saves memory usage and storage space by compressing the data URL.

Example of compression result (number of characters):

google.com:    7265 => 1446
wikipedia.org: 3673 => 1590
github.com:    1306 => 830
altdeus.com:   8808678 => 1830

I was considering replacing the favicon URL with google's favicon API, but rejected it for privacy reasons.

@ngirard
Copy link

ngirard commented May 2, 2021

I've implemented the ability to compress the favicon url when saving the session!

Nice work, thanks ! Will it be on by default ?

@sienori
Copy link
Owner

sienori commented May 2, 2021

@ngirard Yes, it is enabled by default.

@ngirard
Copy link

ngirard commented May 2, 2021

Great, thanks !

@sienori
Copy link
Owner

sienori commented May 2, 2021

In addition, I have published Session Compressor.
You can compress past sessions.

@sienori
Copy link
Owner

sienori commented May 8, 2021

Finally, I implemented it as an option like this
image

@klorinczi
Copy link

klorinczi commented May 8, 2021

Finally, I implemented it as an option like this
image

Great! Thanks!

But does it need a public option?
Is there any reason to turn it off?

How about an option to avoid caching the favicons, just load them from the net in background, when needed?
This would make waaay lower the memory usage.

@sienori
Copy link
Owner

sienori commented May 8, 2021

But does it need a public option?
Is there any reason to turn it off?

Compressing the favicon will make it take longer to save the session.
It takes about 2ms per tab, so 1000 tabs would take 2 seconds (If the favicon is huge, it may take 70ms).
Therefore, users who have a large number of tabs open may want to turn it off.

How about an option to avoid caching the favicons, just load it from the net in background, when needed?
This would make waaay lower the memory usage.

I considered it, but decided against it due to Firefox's specifications...

@klorinczi
Copy link

It takes about 2ms per tab, so 1000 tabs would take 2 seconds (If the favicon is huge, it may take 70ms).

I have almost 1000 tabs.
Speed of loading the tabs is about 30 tabs/second. So loading the 1000 tabs takes at least 33 seconds.
I have to wait 33 seconds to load all tabs before I can do anything, each time.
If I lose the focus over the tab list window, the window closes, and I have to press again the Session Manager button and wait again 33 seconds until the load process finishes.
Very annoying!

I have to wait to load all tabs before the save takes effect after I pressed the save button. Then the save process takes maximum 2 seconds.
The total process to reach the saved state is 35 second.
Well, the maximum 2 second is about 5% of the whole process, so it does not really matter for me.
Tab list loading speed is much higher problem.

I considered it, but decided against it due to Firefox's specifications...

Could you explain?

I would welcome such option to lower memory usage.

@klorinczi
Copy link

Just note, I use the save function to save tabs of Current Window.
Usually I keep one window per subject, so when I finish the search I save tabs of current window for later reference.

@ngirard
Copy link

ngirard commented May 8, 2021

Just note, I use the save function to save tabs of Current Window.

Just curious: which save function are you talking about ? Would you mind posting a screenshot ? I might be missing something here ; I'm using Tab session manager for that purpose.

@ngirard
Copy link

ngirard commented May 8, 2021

It takes about 2ms per tab, so 1000 tabs would take 2 seconds (If the favicon is huge, it may take 70ms).

@sienori, these are interesting numbers, but your sentence sounds as if there's a favicon to save for each tab...
what about tabs with urls from the same domain: do they need one favicon for each tab; or only one ?

@sienori
Copy link
Owner

sienori commented May 8, 2021

@ngirard
The favicon is saved for each tab.

TSM uses browser.tabs.query to get tab information. The favIconUrl of the tab is included here.
In Chrome, favIconUrl is a normal URL like https://example.com/favicon.ico.
But in Firefox, favIconUrl is a data URI like data:image/png;base64,iVBORw0dfXKGgAAAoEAA....
This update compresses the dataURI.

@ngirard
Copy link

ngirard commented May 8, 2021

@sienori, thanks for your detailed reply.

Would it be possible to save a favicon only once when several tabs are visiting the same website ?

@sienori
Copy link
Owner

sienori commented May 8, 2021

@ngirard
Currently it is not possible. And I will not implement it.

There are a lot of things to think about, such as synchronization with other browsers and compatibility with older versions.
Also, the URL may be the same, but the favicon may be different. For example, Discord favicon has a notification badge.

@ngirard
Copy link

ngirard commented May 8, 2021

@sienori, I have no doubt that the problem at hand is not simple. I like to regard our discussion as food for your own thought, rather than a direct request from me.

What about

  • associating a tab to the hash of its favIconUrl instead of the favIconUrl itself,
  • and have a hashmap whose keys are the favIconUrl hashes, and the values are the favIconUrls ?

@sienori
Copy link
Owner

sienori commented May 8, 2021

@ngirard
Hashing the favIconUrl and caching it is a good idea, but...

  • Favicons cannot be displayed without cache
    • When the cache is corrupted
    • When reinstalling TSM and importing the session
    • When importing sessions from another browser with cloud sync
  • Same looking favicon, but favIconUrl can be different
    • Compressing the dataURI will not change the appearance, but it will change the string and hash

By the way, in Chrome, we can access the browser's cache of favicons from chrome://favicon/https://example.com.
We can use this instead of storing the favIconUrl in the session, but this is not implemented in Firefox...

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

No branches or pull requests

7 participants