Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29790626: Noissue - Avoid delete operator on RegExpFilter.regexpSource (Closed)

Created:
May 25, 2018, 10:20 p.m. by Manish Jethani
Modified:
June 4, 2018, 2:21 p.m.
Reviewers:
sergei, kzar
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Deleting a property on V8 hurts performance and memory consumption. In order to understand why, read this post about object properties and elements: https://v8project.blogspot.co.id/2017/08/fast-properties.html This change ensures that the property is not deleted but is merely set to null. In terms of memory consumption, this saves ~10 MB for me (YMMV) with EasyList+AA (~65,000 filters). In terms of performance, the RegExpFilter.regexp getter now runs twice as fast.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M lib/filterClasses.js View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7
Manish Jethani
May 25, 2018, 10:20 p.m. (2018-05-25 22:20:11 UTC) #1
Manish Jethani
Patch Set 1 See description. To test this, export Filter as window.Filter, then in the ...
May 25, 2018, 10:32 p.m. (2018-05-25 22:32:28 UTC) #2
Manish Jethani
I would call changeset 9194ac6bd473 a regression (we made it worse!) so let's fix this ...
May 30, 2018, 12:33 p.m. (2018-05-30 12:33:54 UTC) #3
Manish Jethani
Sergei, do you mind taking a quick look?
June 4, 2018, 1:44 p.m. (2018-06-04 13:44:38 UTC) #4
sergei
Good catch. LGTM.
June 4, 2018, 2:12 p.m. (2018-06-04 14:12:54 UTC) #5
Manish Jethani
Note that in order to see the ~10 MB difference in the heap size you ...
June 4, 2018, 2:17 p.m. (2018-06-04 14:17:00 UTC) #6
Manish Jethani
June 4, 2018, 2:21 p.m. (2018-06-04 14:21:31 UTC) #7
Message was sent while issue was closed.
On 2018/06/04 14:12:54, sergei wrote:
> Good catch. LGTM.

Thanks!

Powered by Google App Engine
This is Rietveld