Skip to content

Commit

Permalink
perf(ivy): improve NaN checks in change detection (#32212)
Browse files Browse the repository at this point in the history
This commit drops our custom, change-detection specific, equality comparison util
in favour of the standard Object.is which has desired semantics.

There are multiple advantages of this approach:
- less code to maintain on our end;
- avoid NaN checks if both values are equal;
- re-write NaN checks so we don't trigger V8 deoptimizations.

PR Close #32212
  • Loading branch information
pkozlowski-opensource authored and AndrewKushnir committed Aug 21, 2019
1 parent 53f33c1 commit 53bfa7c
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 91 deletions.
10 changes: 4 additions & 6 deletions packages/core/src/render3/bindings.ts
Expand Up @@ -12,8 +12,6 @@ import {throwErrorIfNoChangesMode} from './errors';
import {LView} from './interfaces/view';
import {getCheckNoChangesMode} from './state';
import {NO_CHANGE} from './tokens';
import {isDifferent} from './util/misc_utils';



// TODO(misko): consider inlining
Expand All @@ -36,9 +34,11 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any):
ngDevMode && assertNotSame(value, NO_CHANGE, 'Incoming value should never be NO_CHANGE.');
ngDevMode &&
assertLessThan(bindingIndex, lView.length, `Slot should have been initialized to NO_CHANGE`);

const oldValue = lView[bindingIndex];
if (isDifferent(oldValue, value)) {

if (Object.is(oldValue, value)) {
return false;
} else {
if (ngDevMode && getCheckNoChangesMode()) {
// View engine didn't report undefined values as changed on the first checkNoChanges pass
// (before the change detection was run).
Expand All @@ -50,8 +50,6 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any):
lView[bindingIndex] = value;
return true;
}

return false;
}

/** Updates 2 bindings if changed, then returns whether either was updated. */
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/render3/styling_next/util.ts
Expand Up @@ -6,7 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/
import {TNode, TNodeFlags} from '../interfaces/node';
import {isDifferent} from '../util/misc_utils';

import {StylingMapArray, StylingMapArrayIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces';

Expand Down Expand Up @@ -160,7 +159,7 @@ export function hasValueChanged(
if (compareValueB instanceof String) {
compareValueB = compareValueB.toString();
}
return isDifferent(compareValueA, compareValueB);
return !Object.is(compareValueA, compareValueB);
}

/**
Expand Down
12 changes: 0 additions & 12 deletions packages/core/src/render3/util/misc_utils.ts
Expand Up @@ -8,18 +8,6 @@

import {global} from '../../util/global';
import {RElement} from '../interfaces/renderer';
import {NO_CHANGE} from '../tokens';

/**
* Returns whether the values are different from a change detection stand point.
*
* Constraints are relaxed in checkNoChanges mode. See `devModeEqual` for details.
*/
export function isDifferent(a: any, b: any): boolean {
// NaN is the only value that is not equal to itself so the first
// test checks if both a and b are not NaN
return !(a !== a && b !== b) && a !== b;
}

/**
* Used for stringify render output in Ivy.
Expand Down
3 changes: 0 additions & 3 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -1010,9 +1010,6 @@
{
"name": "isDevMode"
},
{
"name": "isDifferent"
},
{
"name": "isFactory"
},
Expand Down
68 changes: 0 additions & 68 deletions packages/core/test/render3/util_spec.ts

This file was deleted.

0 comments on commit 53bfa7c

Please sign in to comment.