Opened on 11/03/2018 at 10:10:52 PM
Closed on 08/29/2019 at 05:43:52 PM
#7097 closed change (rejected)
Make filter objects ephemeral
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Core | Keywords: | closed-in-favor-of-gitlab |
| Cc: | sergz, greiner | Blocked By: | #7029, #7094, #7096, #7452 |
| Blocking: | #7000 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by mjethani)
Background
The next idea to reduce the memory footprint of Adblock Plus is to make the filter objects ephemeral. All Filter objects created are currently held in the Filter.knownFilters map. Of these, about half are not really needed, as they are relatively lightweight and can be created inexpensively on demand.
Consider the following filters:
foo ||bar.com^ ##.foo
The first one is just a literal string (#7052). The second one contains one special character, which currently entails a regular expression, but no other properties. The third one again is just a CSS selector. For filters like these, which make up the vast bulk of filters, there's no need to keep JavaScript objects in memory. An object can be created on demand. In most cases, an object won't even be needed (certainly not in the first and third cases above).
A Filter object could have an ephemeral property. If it is true, Filter.fromText won't keep the object in the Filter.knownFilters map, and the caller will simply discard the object if it doesn't need it beyond its use.
This won't be possible until #7094, #7029, and #7452 are resolved.
It should be noted also that this would break the assumption in the code right now that two calls to Filter.fromText will always return the same object. In order to compare two filters after this change, the code must compare the text rather than the object references.
Based on my experiments, this strategy could save up to 10 MB in memory.
What to change
[...]
Attachments (0)
Change History (12)
comment:2 Changed on 11/03/2018 at 10:18:24 PM by mjethani
- Blocking 7094 added
comment:3 Changed on 11/03/2018 at 10:21:32 PM by mjethani
- Blocked By 7094 added
- Blocking 7094 removed
comment:6 Changed on 11/18/2018 at 03:32:05 AM by mjethani
- Blocked By 7095 removed
- Description modified (diff)
- Summary changed from Make filter objects stateless and ephemeral to Make filter objects ephemeral
comment:7 Changed on 11/18/2018 at 03:38:21 AM by mjethani
- Status changed from new to reviewing
comment:8 Changed on 11/26/2018 at 03:29:26 PM by sergz
- Cc sergz added
comment:9 Changed on 01/17/2019 at 12:15:23 PM by greiner
- Cc greiner added
comment:10 Changed on 04/09/2019 at 02:46:09 AM by mjethani
- Blocked By 7452 added
comment:11 Changed on 04/09/2019 at 02:47:59 AM by mjethani
- Blocked By 7029 added
- Description modified (diff)
comment:12 Changed on 08/29/2019 at 05:43:52 PM by sebastian
- Keywords closed-in-favor-of-gitlab added
- Resolution set to rejected
- Status changed from reviewing to closed
Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.