Opened on 10/14/2018 at 11:47:26 PM

Closed on 08/29/2019 at 05:43:52 PM

#7045 closed change (rejected)

Optimize V8 memory layout of filter text

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: sergz Blocked By:
Blocking: #7000 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29909576/

Description (last modified by mjethani)

Background

In EasyList+AA there are about 150 element hiding filters and exceptions of lengths exceeding 1,000 characters. Of these, about 20 are longer than 100,000 characters. Similarly, there are about 600 blocking and whitelist filters of lengths exceeding 1,000 characters, of which about 10 are longer than 100,000 characters. Most of the filter text is the list of domains, and this list is repeated often.

The memory layout of this text can be optimized by restructuring it in the following manner:

  • Suppose there are two filters example.com#@#selector1 and example.com#@#selector2 occupying a total of 46 bytes
  • First, we slice out the domain example.com (11 bytes) into its own string
  • Next, we slice out the remainders #@#selector1 and #@#selector2 (12 bytes each)
  • Next, we create two new concatenated strings example.com plus #@#selector1 and example.com plus #@#selector2 occupying a total of 35 bytes (11 plus 12 plus 12)

In order for this to work on V8, we would have to override some of V8's own string optimizations that are sometimes harmful (V8 issue #2869). When we "slice" out a part of a string, we must properly separate it from its V8 internal parent string, using an operation like JSON.parse(JSON.stringify(string)). Once we are able to do this, we can restructure such long filter texts with a more efficient layout in memory.

I have written a patch that performs this optimization on filters where the domains part of the text is 1,000 characters or longer. It reduces the initial heap used by ~3.8 MB, from ~39.6 MB to ~35.8 MB. More pertinently, it reduces the memory occupied by all strings from ~16 MB down to ~12 MB.

What to change

[...]

Attachments (0)

Change History (7)

comment:1 Changed on 10/15/2018 at 12:00:05 AM by mjethani

  • Description modified (diff)

comment:2 Changed on 10/15/2018 at 12:00:54 AM by mjethani

  • Blocking 7000 added
  • Component changed from Unknown to Core
  • Priority changed from Unknown to P3
  • Review URL(s) modified (diff)

comment:3 Changed on 10/15/2018 at 05:07:31 AM by mjethani

  • Description modified (diff)

comment:4 Changed on 10/15/2018 at 05:23:17 AM by mjethani

  • Description modified (diff)

comment:5 Changed on 10/15/2018 at 05:25:44 AM by mjethani

  • Owner set to mjethani

comment:6 Changed on 10/15/2018 at 09:54:25 PM by mjethani

  • Cc sergz added

comment:7 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 new to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from mjethani.
 
Note: See TracTickets for help on using tickets.