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

Issue 29933592: Issue 7089 - Match filters by type for non-default types (Closed)

Created:
Nov. 1, 2018, 11:36 p.m. by Manish Jethani
Modified:
Dec. 12, 2018, 12:47 a.m.
Reviewers:
hub
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Memory impact: - Uses ~200 KB more for EasyList+AA Performance impact: - Loading of the default subscriptions may be slightly slower, though I couldn't discern any noticeable/measurable slowdown - No performance impact on filter matching for default types like $script, $image, and so on - Filter matching for non-default types like $elemhide and $csp is about twice as fast now

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -32 lines) Patch
M lib/matcher.js View 3 chunks +139 lines, -32 lines 4 comments Download
M test/matcher.js View 1 chunk +10 lines, -0 lines 2 comments Download

Messages

Total messages: 5
Manish Jethani
Nov. 1, 2018, 11:36 p.m. (2018-11-01 23:36:37 UTC) #1
Manish Jethani
Patch Set 1 https://codereview.adblockplus.org/29933592/diff/29933593/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29933592/diff/29933593/lib/matcher.js#newcode70 lib/matcher.js:70: function* nonDefaultTypes(contentType) For a filter like ...
Nov. 2, 2018, 12:33 a.m. (2018-11-02 00:33:47 UTC) #2
Manish Jethani
For testing this, you could add the following at the end of `lib/matcher.js` along with ...
Nov. 2, 2018, 12:38 a.m. (2018-11-02 00:38:15 UTC) #3
Manish Jethani
We should prioritize this one over the patches for #7094, #7096, and #7097. Reason: Those ...
Nov. 30, 2018, 7:33 a.m. (2018-11-30 07:33:05 UTC) #4
hub
Dec. 11, 2018, 3:34 p.m. (2018-12-11 15:34:59 UTC) #5
LGTM

https://codereview.adblockplus.org/29933592/diff/29933593/lib/matcher.js
File lib/matcher.js (right):

https://codereview.adblockplus.org/29933592/diff/29933593/lib/matcher.js#newc...
lib/matcher.js:316: if ((typeMask & NON_DEFAULT_TYPES) != 0 && (typeMask &
typeMask - 1) == 0)
On 2018/11/02 00:33:47, Manish Jethani wrote:
> The second condition here basically checks if the type mask has only a single
> bit set.

What throw me off here is the evaluation order of `typeMask & typeMask - 1`. But
I get it.

Powered by Google App Engine
This is Rietveld