perf(ivy): improve NaN checks in change detection (#32212)
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
This commit is contained in:
parent
53f33c1cec
commit
53bfa7c6d6
@ -12,8 +12,6 @@ import {throwErrorIfNoChangesMode} from './errors';
|
|||||||
import {LView} from './interfaces/view';
|
import {LView} from './interfaces/view';
|
||||||
import {getCheckNoChangesMode} from './state';
|
import {getCheckNoChangesMode} from './state';
|
||||||
import {NO_CHANGE} from './tokens';
|
import {NO_CHANGE} from './tokens';
|
||||||
import {isDifferent} from './util/misc_utils';
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
// TODO(misko): consider inlining
|
// TODO(misko): consider inlining
|
||||||
@ -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 && assertNotSame(value, NO_CHANGE, 'Incoming value should never be NO_CHANGE.');
|
||||||
ngDevMode &&
|
ngDevMode &&
|
||||||
assertLessThan(bindingIndex, lView.length, `Slot should have been initialized to NO_CHANGE`);
|
assertLessThan(bindingIndex, lView.length, `Slot should have been initialized to NO_CHANGE`);
|
||||||
|
|
||||||
const oldValue = lView[bindingIndex];
|
const oldValue = lView[bindingIndex];
|
||||||
if (isDifferent(oldValue, value)) {
|
|
||||||
|
if (Object.is(oldValue, value)) {
|
||||||
|
return false;
|
||||||
|
} else {
|
||||||
if (ngDevMode && getCheckNoChangesMode()) {
|
if (ngDevMode && getCheckNoChangesMode()) {
|
||||||
// View engine didn't report undefined values as changed on the first checkNoChanges pass
|
// View engine didn't report undefined values as changed on the first checkNoChanges pass
|
||||||
// (before the change detection was run).
|
// (before the change detection was run).
|
||||||
@ -50,8 +50,6 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any):
|
|||||||
lView[bindingIndex] = value;
|
lView[bindingIndex] = value;
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
return false;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Updates 2 bindings if changed, then returns whether either was updated. */
|
/** Updates 2 bindings if changed, then returns whether either was updated. */
|
||||||
|
@ -6,7 +6,6 @@
|
|||||||
* found in the LICENSE file at https://angular.io/license
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
import {TNode, TNodeFlags} from '../interfaces/node';
|
import {TNode, TNodeFlags} from '../interfaces/node';
|
||||||
import {isDifferent} from '../util/misc_utils';
|
|
||||||
|
|
||||||
import {StylingMapArray, StylingMapArrayIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces';
|
import {StylingMapArray, StylingMapArrayIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces';
|
||||||
|
|
||||||
@ -160,7 +159,7 @@ export function hasValueChanged(
|
|||||||
if (compareValueB instanceof String) {
|
if (compareValueB instanceof String) {
|
||||||
compareValueB = compareValueB.toString();
|
compareValueB = compareValueB.toString();
|
||||||
}
|
}
|
||||||
return isDifferent(compareValueA, compareValueB);
|
return !Object.is(compareValueA, compareValueB);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -8,18 +8,6 @@
|
|||||||
|
|
||||||
import {global} from '../../util/global';
|
import {global} from '../../util/global';
|
||||||
import {RElement} from '../interfaces/renderer';
|
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.
|
* Used for stringify render output in Ivy.
|
||||||
|
@ -1010,9 +1010,6 @@
|
|||||||
{
|
{
|
||||||
"name": "isDevMode"
|
"name": "isDevMode"
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"name": "isDifferent"
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"name": "isFactory"
|
"name": "isFactory"
|
||||||
},
|
},
|
||||||
|
@ -1,68 +0,0 @@
|
|||||||
/**
|
|
||||||
* @license
|
|
||||||
* Copyright Google Inc. All Rights Reserved.
|
|
||||||
*
|
|
||||||
* Use of this source code is governed by an MIT-style license that can be
|
|
||||||
* found in the LICENSE file at https://angular.io/license
|
|
||||||
*/
|
|
||||||
|
|
||||||
import {devModeEqual} from '@angular/core/src/change_detection/change_detection_util';
|
|
||||||
import {isDifferent} from '../../src/render3/util/misc_utils';
|
|
||||||
|
|
||||||
describe('util', () => {
|
|
||||||
|
|
||||||
describe('isDifferent', () => {
|
|
||||||
|
|
||||||
describe('checkNoChangeMode = false', () => {
|
|
||||||
it('should mark non-equal arguments as different', () => {
|
|
||||||
expect(isDifferent({}, {})).toBeTruthy();
|
|
||||||
expect(isDifferent('foo', 'bar')).toBeTruthy();
|
|
||||||
expect(isDifferent(0, 1)).toBeTruthy();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should not mark equal arguments as different', () => {
|
|
||||||
const obj = {};
|
|
||||||
expect(isDifferent(obj, obj)).toBeFalsy();
|
|
||||||
expect(isDifferent('foo', 'foo')).toBeFalsy();
|
|
||||||
expect(isDifferent(1, 1)).toBeFalsy();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should not mark NaN as different', () => { expect(isDifferent(NaN, NaN)).toBeFalsy(); });
|
|
||||||
|
|
||||||
it('should mark NaN with other values as different', () => {
|
|
||||||
expect(isDifferent(NaN, 'foo')).toBeTruthy();
|
|
||||||
expect(isDifferent(5, NaN)).toBeTruthy();
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
describe('checkNoChangeMode = true', () => {
|
|
||||||
// Assert relaxed constraint in checkNoChangeMode
|
|
||||||
it('should not mark non-equal arrays, object and function as different', () => {
|
|
||||||
expect(!devModeEqual([], [])).toBeFalsy();
|
|
||||||
expect(!devModeEqual(() => 0, () => 0)).toBeFalsy();
|
|
||||||
expect(!devModeEqual({}, {})).toBeFalsy();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should mark non-equal arguments as different', () => {
|
|
||||||
expect(!devModeEqual('foo', 'bar')).toBeTruthy();
|
|
||||||
expect(!devModeEqual(0, 1)).toBeTruthy();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should not mark equal arguments as different', () => {
|
|
||||||
const obj = {};
|
|
||||||
expect(isDifferent(obj, obj)).toBeFalsy();
|
|
||||||
expect(isDifferent('foo', 'foo')).toBeFalsy();
|
|
||||||
expect(isDifferent(1, 1)).toBeFalsy();
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should not mark NaN as different', () => { expect(isDifferent(NaN, NaN)).toBeFalsy(); });
|
|
||||||
|
|
||||||
it('should mark NaN with other values as different', () => {
|
|
||||||
expect(isDifferent(NaN, 'foo')).toBeTruthy();
|
|
||||||
expect(isDifferent(5, NaN)).toBeTruthy();
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
});
|
|
||||||
|
|
||||||
});
|
|
Loading…
x
Reference in New Issue
Block a user