Possible memory leak in Adblock Plus

[kzar]

Environment

• Chrome 58
• Adlbock Plus 1.13.2.1779
• EasyList Germany+EasyList, Adblock Warning Removal List, AA

How to reproduce

1. Enable those subscriptions, remove any others.
2. Restart the extension from fresh.
3. Take a heap snapshot for the Adblock Plus background page.
4. Wait a while.
5. Take another heap snapshot.

Observed behaviour

Memory usage appears to be growing. While I see 85mb initially, Phill saw around 310mb after some hours of very light usage.

Expected behaviour

Memory usage doesn't grow forever.

[kzar]

We're still not sure that there is a memory leak, but I'm investigating in an attempt to find out.

[sebastian]

BrentM observed that the memory usage, in his case, is going back to normal once he clicks on the browser action icon? Philip, can you check whether this is also true in your case?

[kzar]

FWIW I couldn't reproduce that over the weekend, the memory usage didn't drop down for me after I opened the popup.

[philll]

I can confirm that two of my three ABP instances dropped from ~200MB to ~150MB reported by Chrome's task manager when opening ABP options. The third instance was at around ~150MB and dropped to ~120MB.

[BrentM]

I did another test last night, this time with AdBlock, ABP and uBlock Origin installed, and opened a page to YouTube. YouTube auto-played videos for several hours.

Both AdBlock's and ABP's memory started at about the same amount ~140 MB, and uBlock Origin was around ~58. The interesting thing is that both AdBlock's and ABP's memory climbed throughout the test (the most I saw was around ~430 MB). uBlock Origin's memory never increased beyound 65 MB. I wonder if it's specific extension API that we're using that uBlock Origin is not.

The above test was on a second Windows 10 PC with the stable version of Chrome.

I'll try a similar test on my Mac today, and post the results.

[sebastian]

I was able to reproduce an increase in memory usage from ~120MB to ~160MB after one hour of playing a YouTube playlist, both with the current release version and development build of Adblock Plus. As reported by others the memory usage dropped back to normal after opening the browser action popup.

I took heap snapshots, before and after clicking the browser action icon, and the only noticeable difference I could spot was in the retained size of background page's window object, however, not in any of its contained objects. I filed a ​Chrome bug with more details.

I also tried to record the allocation timeline, however, the memory usage remained stable while the profiler was running. But perhaps somebody else has more luck.

[sebastian]

For reference, Philip confirmed that the memory usage doesn't notably grow on Firefox using our WebExtensions build (which is essentially the same code), which is another indication that something is wrong in Chrome here.

[mapx]

see this post on forum (could be related ?):
​https://adblockplus.org/forum/viewtopic.php?p=169142#p169142

Memory usage issue after windows sleep

After I wake my windows 10 machine from sleep with chrome session still open, the memory usage of the plugin is huge.
Also, I'm using two chrome profiles at the same time and thus there are two adblockplus processes running.

You can see in the image below that it's taking more than 1GB for the two instances of adblock.

Chrome: Version 58.0.3029.110 (64-bit)
Windows 10 64-bit
Active lists: FR+EasyList - Malware Domains - EasyPrivacy

​https://postimg.org/image/zat1f7xm9/

[mapx]

​courtesy of @gorhill:

About https://issues.adblockplus.org/ticket/5291, what I don't see being done in the comment is:

• Open the dev tools
• Go to Memory tab
• Click the trash icon (I do 2-3 times, with a 2-second delay in between).

Yes, I have seen ABP grow to past 340 MB, and even leaving the browser on idle did not cause the memory to be garbage-collected. However, after accomplishing the above steps, there was memory being garbage-collected, and the memory snapped back to expected levels (keeping in mind fragmentation, js engine internals, etc.).

Any report of high memory usage should always be done after the steps above, including the equivalent ones on Firefox.

[kzar]

I haven't had too much luck investigating this so far. Using the memory debugging tools for the background page I don't see memory usage getting too crazy. For example I saved a heap snapshot which showed 95 megabytes of memory being used after the extension had been running for a few days / a week, restarted the extension and saved another which then showed 73 megabytes being used.

I haven't been able to reproduce the garbage collection problem by either opening the extension popup, nor by clicking the trash icon as Gorhill recommended.

Memory usage as listed by the Chrome task manager seems to fluctuate wildly, for example when using the debugging tools it sometimes doubles or worse. So I haven't been able to make much sense of that. I am starting to wonder if it also includes the memory usage of content scripts?

Using the allocation timeline tool I'm seeing memory being used, but not in unexpected ways. Prime culprits there seem to be CombinedMatcher.resultCache in adblockpluscore/lib/matcher.js and Filter.knownFilters in adblockpluscore/lib/filter.js.

So right now I'm trying to figure out what changed in my two snapshots in order to leak that 22 megabytes, but other than that I'm running out of ideas.

[kzar]

Some filters are extremely long, I wonder if we really need to use the whole thing for the knownFilters key? That seems to use a lot of memory, perhaps we could instead use a hash of the filter for the key instead? The parsed filter is stored as the value anyway.

[sebastian]

Replying to kzar:

I haven't had too much luck investigating this so far. Using the memory debugging tools for the background page I don't see memory usage getting too crazy. For example I saved a heap snapshot which showed 95 megabytes of memory being used after the extension had been running for a few days / a week, restarted the extension and saved another which then showed 73 megabytes being used.

Also in my tests, the heap snapshots were much smaller than the memory usage indicated by the task manager, however there still was a notable difference in size of the snapshots, after running Adblock Plus for an extended period, and after triggering the popup. In my tests this difference clearly relates to the retained size of the window object, see above.

Using the allocation timeline tool I'm seeing memory being used, but not in unexpected ways. Prime culprits there seem to be CombinedMatcher.resultCache in adblockpluscore/lib/matcher.js and Filter.knownFilters in adblockpluscore/lib/filter.js.

These caches start off empty when the extension is loaded, and should cause the memory to notably grow after the first few filter matches, but once the caches are populated, the related memory usage should remain stable. I don't see anything suspicious there. But if you compare the memory usage to when the extension just have been loaded, you will obviously notice a difference here.

Replying to kzar:

Some filters are extremely long, I wonder if we really need to use the whole thing for the knownFilters key? That seems to use a lot of memory, perhaps we could instead use a hash of the filter for the key instead? The parsed filter is stored as the value anyway.

If we use a hash as key, the value would need to be an array, holding all filters for the same hash, which we'd need to iterate over and compare the actual filter text, during lookup. Otherwise, we will get (potential) collisions. This is not necessarily any more memory efficient, and also wouldn't explain a memory leak. Furthermore JavaScript engines generally don't copy strings as they are passed around, not exactly sure what happens when using them as key in an object though, but assuming they aren't copied in that scenario either, using a derived key would actually increase memory usage.

[kzar]

Replying to sebastian:

If we use a hash as key, the value would need to be an array, holding all filters for the same hash, which we'd need to iterate over and compare the actual filter text, during lookup. Otherwise, we will get (potential) collisions. This is not necessarily any more memory efficient, and also wouldn't explain a memory leak.

You're right that this doesn't explain the memory leak.

But I think you might be wrong that it wouldn't save memory. I'm seeing over 70 such strings taking over 70kb each. I don't think we would have to always make the value an array, only when there actually was a collision would we have to. I've not tested this idea however, so who knows.

As discussed in IRC I see what you mean now, the Filter Object will have a reference to the string too, so unless a copy is being made this isn't going to help.

[trev]

Replying to kzar:

Some filters are extremely long, I wonder if we really need to use the whole thing for the knownFilters key? That seems to use a lot of memory, perhaps we could instead use a hash of the filter for the key instead? The parsed filter is stored as the value anyway.

This is what we are doing on the Emscripten branch, but I strongly suspect that V8 isn't very different - it shouldn't be creating a new string instance for the hash but rather reuse the same instance. A new instance would only be created if the string is modified which isn't the case here.

[kzar]

I'm struggling to figure this one out, I still plan to investigate but if anyone else has any ideas I'd appreciate a hand.

[mjethani]

Containment view

[mjethani]

Statistics view

[mjethani]

I've attached two screenshots from Developer Tools in Chrome 56.0.2924.0 (base commit a89f0dfb6025b3026dd43dcee02633e502b02b34) on macOS 10.12.5. They're called "Containment view" and "Statistics view". I describe them below.

First I modified the Adblock Plus extension to load the subscriptions every minute instead of after long intervals. I then added five subscriptions. After this I reloaded the extension.

Within a couple of minutes, Chromium's Task Manager showed the extension's memory consumption to be 1.8 GB.

I then took a heap snapshot and examined it in the Containment view. This is the first screenshot. It clearly shows that a lot of memory is being consumed by multiple arrays, all of the same size, and all pointing to the patterns.ini file. Based on my analysis of the code, I conclude that this is the chrome.storage.local.get API simply holding on to the memory it allocates for the file as it is read at regular intervals. This is simply a case of delayed garbage collection. When the popup is opened, the browser seems to run GC and clear up this space. The same effect can be observed by running GC manually.

The second screenshot is simply the same heap snapshot in the Statistics view. It shows clearly that most of the used memory in the heap is being consumed by "System Objects"

I have also found, multiple times, with different versions of Chrome, that eventually the browser does clean up this memory. It may take a long while to do so, but at least on my system it does get around to it eventually.

From this I conclude that this is simply a case of delayed garbage collection. There is no memory leak.

[mjethani]

I should add that I was sent a heap snapshot by philll and that one too similarly implicated patterns.ini

[trev]

Note that browsers generally don't guarantee running garbage collection unless there is memory pressure. However, in the Chrome Developer Tools you have a ​trash can symbol in the Performance tab. This one will let you force garbage collection which should allow validating your claims.

[mjethani]

Yes, I did run GC manually (as I said in my previous comment) by clicking on the trash icon. It had the same effect as when opening the popup.

[rhana@getadblock.com]

Is there an update on this? Thanks.

[sebastian]

It seems there is nothing we can do about this on our end, besides reducing memory usage in general, which would also mitigate the effects of ​delayed garbage collection. Our best attempt here is #4122.

[mjethani]

For what it's worth, there's a ​trick that apparently seems to work for ​memory leaks caused due to V8's "sliced strings"