Issue 6437 - Cache pattern attributes

This builds on https://codereview.adblockplus.org/29713565/

Since pattern objects are immutable, we can cache the values of their computed attributes.

[Manish Jethani, 2018-03-07 17:51:09]

Patch Set 1

[hub, 2018-03-07 19:49:02]

I am of the opinion that this should be part of
https://codereview.adblockplus.org/29712655/

[hub, 2018-03-07 23:11:30]

https://codereview.adblockplus.org/29716641/diff/29716642/lib/content/elemHid...
File lib/content/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29716641/diff/29716642/lib/content/elemHid...
lib/content/elemHideEmulation.js:43: return object[name];
here I would rather do something like:

let value = object[name];
if (value == undefined)
{
   value = defaultValueFunc();
   Object.defineProperty(object, name, {value});
}
return value;

The `in` operator is slower than `[]` and then we avoid a second lookup in the
fast path (hitting the cache).

[Manish Jethani, 2018-03-08 15:29:58]

Patch Set 2: Do not use in operator

https://codereview.adblockplus.org/29716641/diff/29716642/lib/content/elemHid...
File lib/content/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29716641/diff/29716642/lib/content/elemHid...
lib/content/elemHideEmulation.js:43: return object[name];
On 2018/03/07 23:11:30, hub wrote:
> here I would rather do something like:
> 
> let value = object[name];
> if (value == undefined)
> {
>    value = defaultValueFunc();
>    Object.defineProperty(object, name, {value});
> }
> return value;
> 
> The `in` operator is slower than `[]` and then we avoid a second lookup in the
> fast path (hitting the cache).

Thanks, this is a good idea.

Done.

(I'm using strict equality to compare the cached value though since the value
could genuinely be null).

[Manish Jethani, 2018-03-08 15:31:46]

On 2018/03/07 19:49:02, hub wrote:
> I am of the opinion that this should be part of
> https://codereview.adblockplus.org/29712655/

I could merge the two patches, but wouldn't it be too much to review at once?

If you want I could just rebase this one on that one but keep it separate still.

[hub, 2018-03-08 16:39:59]

On 2018/03/08 15:31:46, Manish Jethani wrote:
> On 2018/03/07 19:49:02, hub wrote:
> > I am of the opinion that this should be part of
> > https://codereview.adblockplus.org/29712655/
> 
> I could merge the two patches, but wouldn't it be too much to review at once?
> 
> If you want I could just rebase this one on that one but keep it separate
still.

It wouldn't be too big (I think), but it you prefer like this, it's fine. My
biggest concern is performance, and I had concerns until this. If they all land
together, no one will notice. :-)

[Manish Jethani, 2018-03-08 16:41:01]

On 2018/03/08 16:39:59, hub wrote:
> On 2018/03/08 15:31:46, Manish Jethani wrote:
> > On 2018/03/07 19:49:02, hub wrote:
> > > I am of the opinion that this should be part of
> > > https://codereview.adblockplus.org/29712655/
> > 
> > I could merge the two patches, but wouldn't it be too much to review at
once?
> > 
> > If you want I could just rebase this one on that one but keep it separate
> still.
> 
> It wouldn't be too big (I think), but it you prefer like this, it's fine. My
> biggest concern is performance, and I had concerns until this. If they all
land
> together, no one will notice. :-)

Sounds good, let me merge them then before you take another look.

[Manish Jethani, 2018-03-08 21:09:26]

On 2018/03/08 16:41:01, Manish Jethani wrote:
> On 2018/03/08 16:39:59, hub wrote:
> > On 2018/03/08 15:31:46, Manish Jethani wrote:
> > > On 2018/03/07 19:49:02, hub wrote:
> > > > I am of the opinion that this should be part of
> > > > https://codereview.adblockplus.org/29712655/
> > > 
> > > I could merge the two patches, but wouldn't it be too much to review at
> once?
> > > 
> > > If you want I could just rebase this one on that one but keep it separate
> > still.
> > 
> > It wouldn't be too big (I think), but it you prefer like this, it's fine. My
> > biggest concern is performance, and I had concerns until this. If they all
> land
> > together, no one will notice. :-)
> 
> Sounds good, let me merge them then before you take another look.

Merged into #29712655.