Convert some key null coalescing operators into explicit ternaries

[jacobbergdahl]

Overview

Our nullish coalescing operators are compiled into ternaries that perform unnecessary operations. We got a performance report which indicates that writing explicit ternaries could yield a performance increase. Whatever performance increase this would yield is sure to be slight, but since these are high-volume functions, these changes could still be worthwhile.

Changes

I will use the setupClassUpdate as an example.

Current implementation

export const setupClassUpdate = (className: FacetProp<string | undefined>, element: HTMLElement | SVGElement) => {
  const htmlElement = element as HTMLElement
  if (isFacet(className)) {
    return className.observe((className) => {
      htmlElement.className = className ?? ''
    })
  } else {
    htmlElement.className = className ?? ''
  }
}

... which is compiled into:

const setupClassUpdate = (className, element) => {
    const htmlElement = element;
    if ((0, core_1.isFacet)(className)) {
        return className.observe((className) => {
            htmlElement.className = className !== null && className !== void 0 ? className : '';
        });
    }
    else {
        htmlElement.className = className !== null && className !== void 0 ? className : '';
    }
};

New implementation

export const setupClassUpdate = (className: FacetProp<string | undefined>, element: HTMLElement | SVGElement) => {
  const htmlElement = element as HTMLElement
  if (isFacet(className)) {
    return className.observe((className) => {
      htmlElement.className = className != null ? className : ''
    })
  } else {
    htmlElement.className = className != null ? className : ''
  }
}

... which is compiled into:

const setupClassUpdate = (className, element) => {
    const htmlElement = element;
    if ((0, core_1.isFacet)(className)) {
        return className.observe((className) => {
            htmlElement.className = className != null ? className : '';
        });
    }
    else {
        htmlElement.className = className != null ? className : '';
    }
};

The difference is slight, but the key change is that this line:

htmlElement.className = className !== null && className !== void 0 ? className : '';

... is simplified into this line:

htmlElement.className = className != null ? className : '';

Benchmarking

I benchmarked these changes using the toggleRealisticClass set of tests, using the same command as the CI does:

yarn compare toggleRealisticClassFacet toggleRealisticClassState 45

main

First run

$ yarn compare toggleRealisticClassFacet toggleRealisticClassState 45
Iteration 0:    77910   205794  37.86%
Iteration 1:    77475   168670  45.93%
Iteration 2:    77294   184496  41.89%
Iteration 3:    78214   170316  45.92%
Iteration 4:    77985   180232  43.27%
Iteration 5:    79560   191885  41.46%
Iteration 6:    81160   175491  46.25%
Iteration 7:    77878   180339  43.18%
Iteration 8:    72405   164425  44.04%
Iteration 9:    82503   169012  48.81%
Relative performance toggleRealisticClassFacet/toggleRealisticClassState: 43.69

Second run

$ yarn compare toggleRealisticClassFacet toggleRealisticClassState 45
Iteration 0:    79290   170588  46.48%
Iteration 1:    74512   172083  43.30%
Iteration 2:    75787   181683  41.71%
Iteration 3:    73953   168619  43.86%
Iteration 4:    78395   170952  45.86%
Iteration 5:    72011   181523  39.67%
Iteration 6:    72093   168594  42.76%
Iteration 7:    74099   171172  43.29%
Iteration 8:    75626   171855  44.01%
Iteration 9:    77326   183778  42.08%
Relative performance toggleRealisticClassFacet/toggleRealisticClassState: 43.26

This branch

First run

$ yarn compare toggleRealisticClassFacet toggleRealisticClassState 45
Iteration 0:    87929   184228  47.73%
Iteration 1:    77730   188826  41.16%
Iteration 2:    84099   172100  48.87%
Iteration 3:    81087   167103  48.53%
Iteration 4:    75477   169596  44.50%
Iteration 5:    74574   188689  39.52%
Iteration 6:    75347   181699  41.47%
Iteration 7:    72921   168416  43.30%
Iteration 8:    92224   206429  44.68%
Iteration 9:    84567   229919  36.78%
Relative performance toggleRealisticClassFacet/toggleRealisticClassState: 43.40

Second run

$ yarn compare toggleRealisticClassFacet toggleRealisticClassState 45
Iteration 0:    82056   193020  42.51%
Iteration 1:    81504   185396  43.96%
Iteration 2:    79195   197486  40.10%
Iteration 3:    80108   233137  34.36%
Iteration 4:    130319  276452  47.14%
Iteration 5:    93064   178068  52.26%
Iteration 6:    76055   184336  41.26%
Iteration 7:    73975   163850  45.15%
Iteration 8:    66709   169084  39.45%
Iteration 9:    66828   168137  39.75%
Relative performance toggleRealisticClassFacet/toggleRealisticClassState: 42.58

As you can see, the difference is negligible, but could the result be slightly different in Gameface? Either way, these changes shouldn't possible be able to make anything worse.

Alternatives

This would be a bigger task, but we could instead change the parameters for how we compile our code to begin with, to make a larger-scale change rather than micro-optimizations like in this PR. Whether that is worth the while, however, I cannot say, but the collateral risk would be higher.