Prefer strict equality operator

[mjethani]

Background

Copy and paste the following code into the JavaScript console on both Chrome and Firefox:

(function()
{
  let obj = {};

  let array = new Array(10000000);
  for (let i = 0; i < 10000000; i++)
    array[i] = Math.random() < .5 ? obj : undefined;

  let s = Date.now();
  let n = 0;
  for (let i = 0; i < 10000000; i++)
    if (array[i] == obj)
      n++;
  console.log(Date.now() - s, n);
})();

Change == to === and run the code again.

Now try with strings instead:

(function()
{
  let array = new Array(10000000);
  for (let i = 0; i < 10000000; i++)
    array[i] = Math.random() < .5 ? "foo" : undefined;

  let s = Date.now();
  let n = 0;
  for (let i = 0; i < 10000000; i++)
    if (array[i] == "foo")
      n++;
  console.log(Date.now() - s, n);
})();

Again, change == to === and try again.

In my tests, for the object case, the performance for === (strict equality) is better than for == (non-strict equality) by ~10% on Chrome (V8). On Firefox (SpiderMonkey) it is comparable. For the string case, the performance for === is better by about ~10% on Chrome, while on Firefox it is better by ~25%.

Many if not most ​style guides these days recommend using strict equality by default (e.g. ​Airbnb, ​StandardJS, ​jQuery, etc.).

What to change

Replace instances of == with === and those of != with !==, unless non-strict comparison is required by the logic.

Update ESLint rules if and as necessary.

[mjethani]

@kzar @sergz @sebastian what do you think?

[sergz]

Replying to mjethani:

what do you think?

I am personally used to it, so it's absolutely fine for me even if the performance were equal. Since it differs from mozilla's coding style, I would rather ask someone who is more active in JS than me, perhaps there were reasons for that.

[greiner]

I don't have a strong opinion on that. Although, similar to strict mode, we may not want to apply it to existing code, if we can't reliably detect all instances in which it shouldn't be used.

Therefore, if we want to make any changes, I'd prefer merely changing our coding style to switch the preferred operator from == to ===.

[sebastian]

Currently, our coding style guide prefers == over ===, as per Mozilla's coding style guide which we inherit from.

As for the performance difference, note that in your example you are comparing a value of known type against a value of unknown type. But in the common case when comparing two values they are of known type, and the JIT should be able to optimize the code in same way as if === had been used.

As for maintainability, I prefer == over ===, because it is one less character to type/read, and virtually always has the desired effect. If we'd prefer === there might be potentially situations in which we still have to use ==. Hence it's one more thing to think about when writing code.

[mjethani]

Replying to sebastian:

Currently, our coding style guide prefers == over ===, as per Mozilla's coding style guide which we inherit from.

Yes, but surely that is not cast in stone. If it's better to do things a certain way then we just override that rule in our style guide. The question is whether it's really better. At least my tests suggest that it is.

As for the performance difference, note that in your example you are comparing a value of known type against a value of unknown type. But in the common case when comparing two values they are of known type, and the JIT should be able to optimize the code in same way as if === had been used.

What do you mean? The types of both operands are known. We have lots of these cases, where the type can be either string or undefined (e.g. domain names in a map).

As for maintainability, I prefer == over ===, because it is one less character to type/read, and virtually always has the desired effect. If we'd prefer === there might be potentially situations in which we still have to use ==. Hence it's one more thing to think about when writing code.

That's actually truer in the opposite direction. When using == you have to think about ​type conversion rules, you don't have to think about those rules when using ===, because there is no type conversion.

[sebastian]

Replying to mjethani:

Replying to sebastian:

Currently, our coding style guide prefers == over ===, as per Mozilla's coding style guide which we inherit from.

Yes, but surely that is not cast in stone. If it's better to do things a certain way then we just override that rule in our style guide. The question is whether it's really better. At least my tests suggest that it is.

This was just for reference (so you know where this practice comes from, and as a reminder to update the coding style guide if we agree on changing it).

As for the performance difference, note that in your example you are comparing a value of known type against a value of unknown type. But in the common case when comparing two values they are of known type, and the JIT should be able to optimize the code in same way as if === had been used.

What do you mean? The types of both operands are known. We have lots of these cases, where the type can be either string or undefined (e.g. domain names in a map).

If the type is either a string or undefined then the type isn't known. Anyway, nobody keeps you from using === in cases where it is a (worthwhile) performance improvement.

As for maintainability, I prefer == over ===, because it is one less character to type/read, and virtually always has the desired effect. If we'd prefer === there might be potentially situations in which we still have to use ==. Hence it's one more thing to think about when writing code.

That's actually truer in the opposite direction. When using == you have to think about ​type conversion rules, you don't have to think about those rules when using ===, because there is no type conversion.

That depends on the mindset you follow. If you think in terms of machine instructions, the semantics of === appear simpler. If you just think about two values and want to know if they are equivalent without bothering about the type, the semantics of == appear simpler. While in theory type conversion rules can get complicated, in my experience, == does in practice (virtually) always what you need.

[mjethani]

Replying to sebastian:

Replying to mjethani:

Replying to sebastian:

As for the performance difference, note that in your example you are comparing a value of known type against a value of unknown type. But in the common case when comparing two values they are of known type, and the JIT should be able to optimize the code in same way as if === had been used.

What do you mean? The types of both operands are known. We have lots of these cases, where the type can be either string or undefined (e.g. domain names in a map).

If the type is either a string or undefined then the type isn't known. Anyway, nobody keeps you from using === in cases where it is a (worthwhile) performance improvement.

Thanks, that helps.

I'll wait for @kzar to chime in, but it looks like most of the team agrees that if it helps with performance then it's not a bad idea in general.

[mjethani]

By the way, this issue is only about Core, I'm not suggesting that this be done across all modules. Core obviously has some performance issues, we could use these kinds of small improvements that hopefully add up to something substantial.

[greiner]

Replying to mjethani:

[...] it looks like most of the team agrees that if it helps with performance then it's not a bad idea in general.

For the record, I'm not too concerned with performance at this point because the numbers presented don't show whether comparisons in general have any significant impact on performance in our logic. So while the relative numbers look impressive, the absolute ones may or may not be negligible.
For instance, if it takes us 500ms to determine which filter applies to a request, we may be spending 10ms of that on such comparisons in which case this optimization could save us up to 3ms per request.

What I already do find useful in this proposal though is the fact that it eliminates any ambiguity stemming from type coercion, as was mentioned in this ticket as well. That's usually the reason I hear from people recommending to prefer === over ==.

[mjethani]

Now just to try it out, I ​replaced all instances of == with === in adblockpluscore. There was only one instance that needed to be ==, because one of the operands could be either null or undefined, and I changed it to a ​truthy check instead.

I ran some common tests (ElemHide.getSelectorsForDomain, defaultMatcher.matchesAny, etc.) that I run usually and could not see any noticeable difference in performance. I'm curious if this is any different on mobile.

[sebastian]

Replying to mjethani:

Replying to sebastian:

Anyway, nobody keeps you from using === in cases where it is a (worthwhile) performance improvement.

Thanks, that helps.

Just to clarify, this doesn't mean that I agreed to change our (core) code to use ===/!== everywhere and/or to change our coding style guide, but to keep using ==/!= by default, switching to ===/!== only where it has a relevant impact on the overall performance. However, as per comment:14, there doesn't seem anything to gain either way.

[kzar]

Replying to mjethani:

@kzar @sergz @sebastian what do you think?

FWIW I'd prefer we switch to strict equality by default anyway. To my mind we should always check things are strictly equal, unless we specifically want the type conversion which == gives. I never really understood why we did it the other way around at eyeo.

[agiammarchi]

FWIW I'm OK with === but there is the null case that is special when both undefined and null might be the desired/undesired value, and nothing else.

Example: you cannot always replace null == value with !value because 0, or empty strings, fails == null comparison but pass the !value one, so there might be side effects.

In that specific case in comment:14, since it's expected either a node or nothing * I don't see any side effect, but writing value === null || value === undefined when per specs, and since ever, is the exact equivalent of value == null, looks a bit silly to me.

This is why the linter has a rule to allow value == null specifically for the null case.

---

* I wonder why one would ever makeSelector(null) that also returns null ... if you need to check the result, you are probably better off not calling that function in the first place by checking directly node

[mjethani]

Replying to sebastian:

As for maintainability, I prefer == over ===, because it is one less character to type/read, and virtually always has the desired effect. If we'd prefer === there might be potentially situations in which we still have to use ==. Hence it's one more thing to think about when writing code.

I have to admit that while making the changes for the ​patch I realized that the cognitive overhead of deciding between == and === may not be worth it. In most cases, where we want to compare with null, we also want to compare with undefined; on the other hand, like you said, == almost always does the right thing, hence imposes little cognitive overhead. But I do wonder if the latter is the case only because it is already the status quo in our codebase.

There may be arguments on the other side as well, but I personally don't feel strongly about this any more.

[mjethani]

I am closing this now because after one year of working on adblockpluscore full time I am convinced that there is no need to do this.