Opened on 09/19/2017 at 12:31:10 PM
Closed on 09/26/2017 at 03:19:33 PM
Last modified on 01/05/2018 at 04:03:04 PM
#5735 closed change (fixed)
Use JS Map instead of Object
Reported by: | sergz | Assignee: | sergz |
---|---|---|---|
Priority: | P3 | Milestone: | |
Module: | Core | Keywords: | |
Cc: | trev, fhd | Blocked By: | |
Blocking: | Platform: | Unknown / Cross platform | |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
https://codereview.adblockplus.org/29550598/ |
Description (last modified by sergz)
Background
I was profiling memory usage of current adblockpluscore a little bit and discovered that after the changes described below the size of RSS is 10-15 MBs less for EasyList+AA, in particular instead of 80.+ MB originally, it's 70.- MBs with the changes (it's a memory usage of a special tests, not all functionality is there). Each change saves merely a few hundreds of KBs or a couple of MBs but in total it's a noticeable result.
I have also tried to change some other Objects which are used as containers to JS Map but it had not helped, so only these changes have a positive effect, at least in my tests.
What to change
Make to be Map:
- Filter.knownFilters
- Filter property domains after lazy evaluation
- Matcher property filterByKeyword
- Matcher property keywordByFilter
- in elemHide.js
- global keyByFilter
- global filtersByDomain
- filters which are stored in filtersByDomain
Integration notes
Please ensure in the dependent projects that the objects mentioned above are treated in accordance with the changes. In particular, the type of following objects changed from Object to Map:
- Filter.knownFilters
- domains property of Filter objects
- Matcher properties filterByKeyword and keywordByFilter
- global (module scoped) filtersByDomain and its values in elemHide.js
- global (module scoped) keyByFilter in elemHide.js
Attachments (0)
Change History (10)
comment:1 Changed on 09/19/2017 at 12:49:11 PM by trev
- Priority changed from Unknown to P3
- Ready set
comment:5 Changed on 09/20/2017 at 01:04:07 PM by sergz
comment:6 Changed on 09/21/2017 at 12:02:14 PM by sergz
comment:7 Changed on 09/21/2017 at 01:24:14 PM by sergz
comment:8 Changed on 09/26/2017 at 01:56:55 PM by abpbot
comment:9 Changed on 09/26/2017 at 03:19:33 PM by sergz
- Resolution set to fixed
- Status changed from new to closed
comment:10 Changed on 01/05/2018 at 04:03:04 PM by kzar
(Also see the discussion in #3095.)
Some commits referencing this issue have landed: