Opened 13 months ago

Closed 3 months ago

#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):

https://codereview.adblockplus.org/29945631/

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

See #7094, #7029, and #7452.

[...]

Change History (12)

comment:1 Changed 13 months ago by mjethani

  • Description modified (diff)

comment:2 Changed 13 months ago by mjethani

  • Blocking 7094 added

comment:3 Changed 13 months ago by mjethani

  • Blocked By 7094 added
  • Blocking 7094 removed

comment:4 Changed 13 months ago by mjethani

  • Description modified (diff)

comment:5 Changed 12 months ago by mjethani

  • Review URL(s) modified (diff)

comment:6 Changed 12 months ago 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 12 months ago by mjethani

  • Status changed from new to reviewing

comment:8 Changed 12 months ago by sergz

  • Cc sergz added

comment:9 Changed 10 months ago by greiner

  • Cc greiner added

comment:10 Changed 7 months ago by mjethani

  • Blocked By 7452 added

comment:11 Changed 7 months ago by mjethani

  • Blocked By 7029 added
  • Description modified (diff)

comment:12 Changed 3 months ago 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.

Note: See TracTickets for help on using tickets.