Svelte: simplify safe_not_equal and not_equal?

Created on 27 May 2019  Â·  7Comments  Â·  Source: sveltejs/svelte

I was doing some profiling the other day and was mildly surprised to find that a lot of the time spent in Svelte — possibly even most? — was in safe_not_equal. It occurs to me that we might see some performance gains by changing the implementations thusly:

+function is_mutable(type) {
+   return type === 'object' || type === 'function';
+}
+
export function safe_not_equal(a, b) {
-   return a != a ? b == b : a !== b || ((a && typeof a === 'object') || typeof a === 'function');
+   return a !== b || a && is_mutable(typeof a);
}

export function not_equal(a, b) {
-   return a != a ? b == b : a !== b;
+   return a !== b;
}

The downside is that setting x to NaN would trigger an update if x was already NaN. But I'm pretty sure that's a small enough edge case that we needn't worry about it

internals

Most helpful comment

Just a question. The most important thing to optimize is when they are equal, correct? Because if they’re not, that leads to an update that probably makes any microoptimization dwarf in comparison? I might be missing something, but if that’s true, there’s no reason to do a NaN check when a === b anyway, and little harm in doing it when a !== b?

All 7 comments

This seems reasonable.

Is there much of a performance gain in putting is_mutable into its own function? Does losing one typeof and gaining a function call help?

I am not arguing. But, I would suggest keeping the method not_equal as it is to handle NaN as this is a special case may raise a performance issue if missed.

I have changed fnis_mutable little bit to accept an object instead of the typeof of the same as this makes sense of this case.

export function is_mutable(obj) {
  return typeof obj === 'object' || typeof obj === 'function'
}

export function safe_not_equal(a, b) {
  return not_equal(a, b) || is_mutable(a)
}

export function not_equal(a, b) {
  return a != a ? b == b : a !== b;
}

Using this snippet...

const obj = {};
const foo = function () { }

function safe_not_equal(a, b) {
    return a != a ? b == b : a !== b || ((a && typeof a === 'object') || typeof a === 'function');
}

function safe_not_equal_simplified(a, b) {
    return a !== b || (a && (typeof a === 'object' || typeof a === 'function'));
}

function is_mutable(type) {
    return type === 'object' || type === 'function';
}

function safe_not_equal_simplified_indirect(a, b) {
    return a !== b || (a && is_mutable(typeof a));
}

// --------------------

const iterations = 1e7;

function test(fn, a, b) {
    console.time(fn.name);
    let i = iterations;
    while (i--) {
        const object_differs = fn(a, b);
    }
    console.timeEnd(fn.name);
}

...then running things like test(safe_not_equal, obj, obj), I get a clear answer — safe_not_equal_simplified is the quickest. It seems that the function call overhead is more significant than I imagined at this level of microoptimisation — the last version (safe_not_equal_simplified_indirect) is a little bit slower in Chrome, and an extraordinary amount slower in Firefox.

@karuppasamy The whole point of this issue is to increase performance! As discussed, NaN is a rare edge case. Your implementation makes everything slower — it keeps the NaN check but adds a function call, keeps the is_mutable function call and keeps the double typeof check.

Just a question. The most important thing to optimize is when they are equal, correct? Because if they’re not, that leads to an update that probably makes any microoptimization dwarf in comparison? I might be missing something, but if that’s true, there’s no reason to do a NaN check when a === b anyway, and little harm in doing it when a !== b?

@trbrc to be clear, is this what you're proposing?

function safe_not_equal(a, b) {
    return a === b
        ? (a ? (typeof a === 'object' || typeof a === 'function') : false)
        : a === a && b === b;
}

@Rich-Harris Yes, something very much like it. I have no idea where it sits with real world performance, but it looks to me like it should keep the original semantics but with less work to return false, and very similar work to return true. Do you agree?

@trbrc's proposal is solid, imo. You just got a little bug with NaNs, but once corrected it is the overall winner on my machine.

I also tried it without the polymorphic test(fn) since Svelte always references the same function throughout, and was surprised by how much close they all are in FF on Windows for me.

Little live test case if anyone also wants to check it out:

Was this page helpful?
0 / 5 - 0 ratings