Noissue - Remove ElemHideBase.selectorDomains

Noissue - Remove ElemHideBase.selectorDomains

[Manish Jethani, 2018-05-24 03:04:31]

Patch Set 1

I'm trying to get rid of unnecessary memory usage in Core, especially the filter
classes. I see that ElemHideBase.selectorDomains is not being used at all in the
web extension. We generate this string for each element hiding and emulation
filter/exception even though it is only used for testing.

So I'm making this a getter, which will generate a new array when called. The
unit tests are happy with this with no changes.

[Manish Jethani, 2018-05-24 03:05:37]

On 2018/05/24 03:04:31, Manish Jethani wrote:
> Patch Set 1
> 
> I'm trying to get rid of unnecessary memory usage in Core, especially the
filter
> classes. I see that ElemHideBase.selectorDomains is not being used at all in
the
> web extension. We generate this string for each element hiding and emulation
> filter/exception even though it is only used for testing.
> 
> So I'm making this a getter, which will generate a new array when called. The
> unit tests are happy with this with no changes.

FWIW I don't even mind removing property this entirely. It would simplify the
Snippets implementation for one.

[kzar, 2018-05-24 13:36:28]

If selectorDomains is only being used for the tests (and looking at the code I
can't see where else it is) then my vote is to just get rid of it. Well spotted.

[Manish Jethani, 2018-05-25 03:38:32]

Patch Set 2: Delete selectorDomains

In terms of memory consumption, this doesn't help too much, but it does help.
For me it saved ~500 KB after the extension is loaded in Chrome with EasyList+AA
(~69,000 filters). The number of internal string objects in V8 went from
~290,000 to ~285,000, and the number of "sliced strings" from ~117,000 to
~110,000. It's small, but we have to take these small steps.

On 2018/05/24 13:36:28, kzar wrote:
> If selectorDomains is only being used for the tests (and looking at the code I
> can't see where else it is) then my vote is to just get rid of it. Well
spotted.

OK, let's remove it then.

Done.

[Manish Jethani, 2018-05-25 03:40:38]

Sergei, is it OK to remove this property entirely from the point of view of
libadblockplus and its clients?

[kzar, 2018-05-25 13:44:03]

LGTM

[sergei, 2018-05-28 09:27:02]

On 2018/05/25 03:40:38, Manish Jethani wrote:
> Sergei, is it OK to remove this property entirely from the point of view of
> libadblockplus and its clients?

Yes. At least I didn't discover any usage of it.