philc / vimium

The hacker's browser.
https://chrome.google.com/webstore/detail/vimium/dbepggeogbaibhgnhhndojpepiihcmeb
MIT License
23.62k stars 2.49k forks source link

memory consumption too much? #2055

Closed dragonxlwang closed 8 years ago

dragonxlwang commented 8 years ago

just wanna know if this is normal that with around 10 tabs open, the vimium shows up in task manager to take up to 600MB memory. After turn it off, the memory released again. and turn it on once more, it drops to around 200MB after a while.

smblott-github commented 8 years ago

@dragonxlwang... It's not normal...

snapshot

That's a Vimium instance that's been running for days with many tabs.

Things to check... Do you have any non-standard Chrome flags set? What happens if you run Vimium in a new (clean) Chrome account (e.g., without other extensions installed)?

See also #892.

dragonxlwang commented 8 years ago

Thanks for the feed. I would try it for a while with disabling all other plugin resetting everything. It does seem like a memory leak as I can always have memory drop back after a restart

Here's what it looks like after running for half day:

image

Here's what it looks like after a restart image

laertis commented 8 years ago

Hello, I've been having the same issue but my memory consumption goes up to 160 MB after days of extensive use which is still large. I suspect that if the extension doesn't leak any memory it is a caching issue. Running a clean chrome without extensions installed is not a valid/realistic option

smblott-github commented 8 years ago

it is a caching issue.

Could you elaborate, @laertis?

brianvanburken commented 8 years ago

I noticed the same issues. Today I tried to not use any of the functions to see what the result would be. Not using anything still results in a ~600MB memory consumption.

screen shot 2016-03-21 at 15 25 33
smblott-github commented 8 years ago

@brianvanburken. This is weird. Could you confirm that the actual process itself is really using that much memory?

Also, I notice that the "memory consumption too much" tab you're showing is 336MB. For me it's 60MB. Could that be right too? Could this just be some dodgy Chrome metrics?

brianvanburken commented 8 years ago

I am currently trying to debug where the issue is coming from.

When looking into "Inspect views" I see a lot of errors for Icon invalid.

screen shot 2016-03-21 at 15 38 15

uBlock origin and other extensions also add more memory each to a tab. That could explain why the "memory consumption too much" tab is using more memories.

Also I currently am using the Google Chrome Beta version 49.0.2623.87 (64-bit).

smblott-github commented 8 years ago

I see a lot of errors for Icon invalid.

That's #2040 -- which is fixed in master, but not yet released.

brianvanburken commented 8 years ago

Oke, so I am able to replicate the memory growth by starting a fresh instance of Google Chrome 49.0.2623.87 (64-bit) and open 20+ github repository tabs and closing them one by one. I see that the memory consumption grows with each tab opened, but when I close the tab the memory is not released and reached 120 MB. After killing all but one "about:blank" page I waited a few minutes to see if it would release the memory. It didn't. So after a day browsing with vimium active it adds up untill the big numbers like 600MB+ as mentioned above.

Don't know if this is due to Google Chrome beta?

smblott-github commented 8 years ago

Edit... This post is nonsense. Please ignore it.

OK. I tried Chrome Beta (50.0.2661.37-1) and I can replicate the problem there (with both 1.54 and master). I cannot replicate it with Chrome 47.0.2526.111.

The problem seems to be related to ports (chrome.runtime.connect()). If we allow the ports to connect, and allow messages to pass from page to background page, but not from background page to page, then all is fine. However, any message from the background page to the page seems to cause a memory leak (even if we just discard the request on the other end).

For the moment, I'm stumped. Re-reading the documentation, there doesn't seem to be anything wrong with our use of ports.

Incidentally, since cVim is similar to Vimium, I installed that, and it seems to have the same problem.

Edit: Closed accidentally.

smblott-github commented 8 years ago

Please ignore that last comment. It is/was bogus.

This commit 15400b1bc30d47c6cbeabc77cde2b776fe4c13e2 seems to fix the problem. (This just never updates the icon.)

It seems to be related to the icon.

smblott-github commented 8 years ago

I've submitted a Chromium Bug for this (here). If you're affected, please go over there and star the issue - so they know.

In the meantime, we need a workaround, and it's possible that there are other effects at play.

smblott-github commented 8 years ago

Update...

After upgrading to 49.0.2623.87 (stable), I'm seeing this issue too.

Also, Vimium sets tabId-specific icons. I originally thought that we could work around this by not specifying a tabId. It turns out that that's not the case. Simply setting an icon repeatedly triggers the problem. (It looks like Adblock Plus has the same problem.)

Possible workarounds:

  1. Only set the icon when it changes. If we assume that Vimium is enabled in most tabs, then most of the time it doesn't change. We'd still have a memory leak, but it would be a slower leak.
  2. (Nobody's going to like this...) Temporarily use badges instead of an icon to indicate the enabled state.

I'm going to go ahead and implement 1.

@philc... We might need to push a new release because of this.

smblott-github commented 8 years ago

See a0b290100889ce02b540931676bfd6f7eb1f2d37. This is the best (partial) workaround I can think of for the moment.

smblott-github commented 8 years ago

Update...

Try this experiment (you'll need the patience of a saint, or - perhaps, just take my word for it):

  1. Find a quick-loading page (such as this one).
  2. 100yt, then (individually!) x, x, x, until you're back to just one tab again.
  3. Repeat step 2 many times, watching the extension's memory in the task manager.

Here is a list of the high-water marks and low-water marks (MB):

Conclusion... It seems the memory leak is not quite a serious as (I) thought. The initial memory increase looks scary, but after some time Chrome seems to recover.

Some of the reports here quote extraordinarily large memory consumptions (e.g. @brianvanburken). I wonder whether there might be other effects at play -- @brianvanburken... the numbers you show are unusually large across the board, not just for Vimium.

Proposal... Don't panic. We need a better understanding of what's going on before acting. If anybody else has more time than sense, then it would be great to get independent verification of the results above.

(Separately, I tried setting the icon via imageData (as opposed via a path), a385e1cbb3a699b6aa4d32152029c607aa352bde. The behaviour in this case is the same as described here, although the magnitude is smaller, probably just because the icons themselves are smaller.)

(In step 2 above, tabs must be removed individually because we do not update the icon until a page receives the focus; so 100yt, 100x doesn't trigger the issue.)

brianvanburken commented 8 years ago

@smblott-github I'll pull your branch and test it locally on my machine to test it today.

Also I'm puzzled myself by the high memory usage. I've always considered this normal. I've tried to lower it by reseting the flags to default, disabling all extension and plugins, reinstalling chrome and resetting the settings. None seem to work. I guess this is related to me using the beta or an other external factor.

smblott-github commented 8 years ago

@brianvanburken I was testing master. I noticed the increase-then-drop-back behaviour testing a385e1cbb3a699b6aa4d32152029c607aa352bde, so I went back to master to see if the same thing happened there.

brianvanburken commented 8 years ago

@smblott-github I've used your branch containing a385e1c all day today. I've seen it being stable on ~80mb memory usage for me. Seems like your workaround fixes the memory leak so far.

smblott-github commented 8 years ago

I think we'll probably go with #2070 for fixing this. It's a variation on a385e1cbb3a699b6aa4d32152029c607aa352bde which seems to work for both @brianvanburken and me.