Opened on 09/28/2018 at 11:04:23 PM
Closed on 09/29/2018 at 12:45:43 AM
Last modified on 10/17/2018 at 12:40:35 PM
#7001 closed defect (fixed)
CombinedMatcher result cache keeps growing indefinitely
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P1 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | jsonesen, hfiguiere, sebastian | Blocked By: | |
| Blocking: | #6741, #7002 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by mjethani)
Environment
ABP 3.4 pre-release development build
How to reproduce
- Reload the extension and start loading different websites in new tabs
- Open the background page in DevTools and take a heap snapshot
- Look for the Map object called resultCache
Observed behaviour
The object may contain more than 1,000 entries.
Expected behaviour
The object should contain no more than 1,000 entries.
Additional notes
This is a P1 regression caused by 84ea8a5e3951.
Hints for testers
It's not really feasible to test this using the extension.
If you would like, make a web page like this:
<!DOCTYPE html>
<script>
setTimeout(() =>
{
let urls = [];
for (let i = 0; i < 100000; i++)
urls.push("http://localhost/iny6703v26m/evnuqh1a4ak/qicz2zyn47/doubleclick/300x500/ads?index=" + i);
console.log("Test started.");
Promise.all([urls.map(url => fetch(url))]).then(() =>
{
console.log("Test completed.");
});
});
</script>
Now load the page and wait ... if the memory usage keeps going up, the bug isn't fixed. After this fix, the memory should stay somewhat constant even for 100,000 URL requests.
Attachments (0)
Change History (7)
comment:1 Changed on 09/28/2018 at 11:05:42 PM by mjethani
- Cc jsonesen hfiguiere sebastian added
- Description modified (diff)
- Owner set to mjethani
comment:2 Changed on 09/28/2018 at 11:11:36 PM by mjethani
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 09/29/2018 at 12:30:14 AM by abpbot
comment:4 Changed on 09/29/2018 at 12:31:41 AM by abpbot
A commit referencing this issue has landed:
Issue 7001 - Use instance property maxCacheEntries in CombinedMatcher
comment:5 Changed on 09/29/2018 at 12:45:43 AM by mjethani
- Description modified (diff)
- Resolution set to fixed
- Status changed from reviewing to closed
comment:6 Changed on 09/29/2018 at 12:46:06 AM by mjethani
- Blocking 7002 added
comment:7 Changed on 10/17/2018 at 12:40:35 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Fixed. ABP memory usage stays somewhat constant, hovering around 100-130MB using the test HTML above.
ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10
A commit referencing this issue has landed:
Issue 7001 - Use instance property maxCacheEntries in CombinedMatcher