Issue 6665 - Optimize element hiding emulation filter lookups

[Manish Jethani, 2018-05-13 20:39:48]

Patch Set 1

This patch speeds up element hiding emulation filter lookups by domain. Ideally
we would merge ElemHide and ElemHideEmulation, but that's a more difficult task.
The changes here are relatively simple (the main parts are copied from
ElemHide).

I tested with a filter list containing 50 unique domains (each one has some
element hiding emulation filters) and the speedup is like ~2.5x, which is nice.
There are ~11,500 unique domains for element hiding in EasyList+AA. If the usage
of element hiding emulation grows (as I hope it does), the current code will not
scale well. We have to split up the filters by domain eventually.

https://codereview.adblockplus.org/29780560/diff/29780565/lib/coreUtils.js
File lib/coreUtils.js (right):

https://codereview.adblockplus.org/29780560/diff/29780565/lib/coreUtils.js#ne...
lib/coreUtils.js:59: function* filterIt(iterable, callback, thisArg)
These are helper functions as described in Trac #6434. They help encapsulate
common logic (filtering, mapping, flattening, etc.) for non-array iterables.

https://codereview.adblockplus.org/29780560/diff/29780565/lib/elemHideEmulati...
File lib/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29780560/diff/29780565/lib/elemHideEmulati...
lib/elemHideEmulation.js:125: let excludeSet = new Set();
The logic here is similar to lib/elemHide.js

Eventually we can merge the two modules, but that will be more work. The changes
here are relatively simple so we can do this first.

[Manish Jethani, 2018-05-13 20:50:54]

Patch Set 2: Separate out subdomains generator function
Patch Set 3: Update JSDoc
Patch Set 4: Fix JSDoc yield type

[Manish Jethani, 2018-05-14 01:22:27]

Patch Set 5: Roll back generics overkill

I'll settle for for loops, since generics seem to affect performance slightly,
especially when they're nested like flatten(map(...))

[Manish Jethani, 2018-05-14 02:25:07]

Patch Set 6: Use String.indexOf for subdomains

After some testing I find that the simple approach of using String.indexOf is a
lot faster.

[hub, 2018-05-14 20:25:59]

Issue #6665 is about merging ElemHide and ElemHideEmulation. It this related to
that issue?

[Manish Jethani, 2018-05-15 11:05:41]

On 2018/05/14 20:25:59, hub wrote:
> Issue #6665 is about merging ElemHide and ElemHideEmulation. It this related
to
> that issue?

So the main reason for merging the modules is to get the performance benefits of
ElemHide. This is an alternative way of doing it (without merging entirely). If
we want to target the Adblock Plus 3.2 release, we could do something like this
first; then later we could try to fully merge the modules (at that point it
would become more of a code maintenance task).

I feel like this is the more practical approach, as a start.

[kzar, 2018-05-15 11:18:17]

NOT LGTM, I don't like that we're duplicating the logic here and would rather we
instead shared it if possible.

[Manish Jethani, 2018-05-15 12:06:55]

On 2018/05/15 11:18:17, kzar wrote:
> NOT LGTM, I don't like that we're duplicating the logic here and would rather
we
> instead shared it if possible.

Alright, in that case I'll wait for the other changes to lib/elemHide.js to land
before trying to share code between the two modules or just merge them entirely.