refactor(ivy): move all of the instruction state into a singe object (#33093)

Turns out that writing to global state is more expensive than writing to
a property on an object.

Slower:
````
let count = 0;

function increment() {
  count++;
}
```

Faster:
````
const state = {
  count: 0
};

function increment() {
  state.count++;
}
```

This change moves all of the instruction state into a single state object.

`noop_change_detection` benchmark
Pre refactoring: 16.7 us
Post refactoring: 14.523 us (-13.3%)

PR Close #33093
This commit is contained in:
Miško Hevery 2019-10-11 12:43:32 -07:00 committed by Matias Niemelä
parent 43487f6761
commit bb53b6549c
6 changed files with 214 additions and 184 deletions

View File

@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime": 1440,
"main": 13415,
"main": 14228,
"polyfills": 45340
}
}
@ -21,7 +21,7 @@
"master": {
"uncompressed": {
"runtime": 1440,
"main": 123904,
"main": 125674,
"polyfills": 45340
}
}

View File

@ -184,7 +184,7 @@ function stylingProp(
// it's important we remove the current style sanitizer once the
// element exits, otherwise it will be used by the next styling
// instructions for the next element.
setElementExitFn(resetCurrentStyleSanitizer);
setElementExitFn(stylingApply);
}
} else {
// Context Resolution (or first update) Case: save the value
@ -354,7 +354,7 @@ function _stylingMap(
// it's important we remove the current style sanitizer once the
// element exits, otherwise it will be used by the next styling
// instructions for the next element.
setElementExitFn(resetCurrentStyleSanitizer);
setElementExitFn(stylingApply);
}
} else {
updated = valueHasChanged;

View File

@ -7,67 +7,185 @@
*/
import {StyleSanitizeFn} from '../sanitization/style_sanitizer';
import {assertDefined} from '../util/assert';
import {assertDefined, assertEqual} from '../util/assert';
import {assertLViewOrUndefined} from './assert';
import {ComponentDef, DirectiveDef} from './interfaces/definition';
import {TElementNode, TNode, TViewNode} from './interfaces/node';
import {CONTEXT, DECLARATION_VIEW, LView, OpaqueViewState} from './interfaces/view';
/**
* Store the element depth count. This is used to identify the root elements of the template
* so that we can than attach `LView` to only those elements.
* All implicit instruction state is stored here.
*
* It is useful to have a single object where all of the state is stored as a mental model
* (rather it being spread across many different variables.)
*
* PERF NOTE: Turns out that writing to a true global variable is slower than
* having an intermediate object with properties.
*/
let elementDepthCount !: number;
interface InstructionState {
/**
* State of the current view being processed.
*
* An array of nodes (text, element, container, etc), pipes, their bindings, and
* any local variables that need to be stored between invocations.
*/
lView: LView;
/**
* Used to set the parent property when nodes are created and track query results.
*
* This is used in conjection with `isParent`.
*/
previousOrParentTNode: TNode;
/**
* If `isParent` is:
* - `true`: then `previousOrParentTNode` points to a parent node.
* - `false`: then `previousOrParentTNode` points to previous node (sibling).
*/
isParent: boolean;
/**
* Index of currently selected element in LView.
*
* Used by binding instructions. Updated as part of advance instruction.
*/
selectedIndex: number;
/**
* The last viewData retrieved by nextContext().
* Allows building nextContext() and reference() calls.
*
* e.g. const inner = x().$implicit; const outer = x().$implicit;
*/
contextLView: LView;
/**
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
*
* Necessary to support ChangeDetectorRef.checkNoChanges().
*/
checkNoChangesMode: boolean;
/**
* Store the element depth count. This is used to identify the root elements of the template
* so that we can then attach `LView` to only those elements.
*/
elementDepthCount: number;
/**
* Stores whether directives should be matched to elements.
*
* When template contains `ngNonBindable` then we need to prevent the runtime form matching
* directives on children of that element.
*
* Example:
* ```
* <my-comp my-directive>
* Should match component / directive.
* </my-comp>
* <div ngNonBindable>
* <my-comp my-directive>
* Should not match component / directive because we are in ngNonBindable.
* </my-comp>
* </div>
* ```
*/
bindingsEnabled: boolean;
/**
* Current namespace to be used when creating elements
*/
currentNamespace: string|null;
/**
* Current sanitizer
*/
currentSanitizer: StyleSanitizeFn|null;
/**
* Used when processing host bindings.
*/
currentDirectiveDef: DirectiveDef<any>|ComponentDef<any>|null;
/**
* Used as the starting directive id value.
*
* All subsequent directives are incremented from this value onwards.
* The reason why this value is `1` instead of `0` is because the `0`
* value is reserved for the template.
*/
activeDirectiveId: number;
/**
* The root index from which pure function instructions should calculate their binding
* indices. In component views, this is TView.bindingStartIndex. In a host binding
* context, this is the TView.expandoStartIndex + any dirs/hostVars before the given dir.
*/
bindingRootIndex: number;
/**
* Current index of a View or Content Query which needs to be processed next.
* We iterate over the list of Queries and increment current query index at every step.
*/
currentQueryIndex: number;
/**
* Function to be called when the element is exited.
*
* NOTE: The function is here for tree shakable purposes since it is only needed by styling.
*/
elementExitFn: (() => void)|null;
}
export const instructionState: InstructionState = {
previousOrParentTNode: null !,
isParent: null !,
lView: null !,
// tslint:disable-next-line: no-toplevel-property-access
selectedIndex: -1 << ActiveElementFlags.Size,
contextLView: null !,
checkNoChangesMode: false,
elementDepthCount: 0,
bindingsEnabled: true,
currentNamespace: null,
currentSanitizer: null,
currentDirectiveDef: null,
activeDirectiveId: 0,
bindingRootIndex: -1,
currentQueryIndex: 0,
elementExitFn: null,
};
export function getElementDepthCount() {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return elementDepthCount;
return instructionState.elementDepthCount;
}
export function increaseElementDepthCount() {
elementDepthCount++;
instructionState.elementDepthCount++;
}
export function decreaseElementDepthCount() {
elementDepthCount--;
instructionState.elementDepthCount--;
}
let currentDirectiveDef: DirectiveDef<any>|ComponentDef<any>|null = null;
export function getCurrentDirectiveDef(): DirectiveDef<any>|ComponentDef<any>|null {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return currentDirectiveDef;
return instructionState.currentDirectiveDef;
}
export function setCurrentDirectiveDef(def: DirectiveDef<any>| ComponentDef<any>| null): void {
currentDirectiveDef = def;
instructionState.currentDirectiveDef = def;
}
/**
* Stores whether directives should be matched to elements.
*
* When template contains `ngNonBindable` than we need to prevent the runtime form matching
* directives on children of that element.
*
* Example:
* ```
* <my-comp my-directive>
* Should match component / directive.
* </my-comp>
* <div ngNonBindable>
* <my-comp my-directive>
* Should not match component / directive because we are in ngNonBindable.
* </my-comp>
* </div>
* ```
*/
let bindingsEnabled !: boolean;
export function getBindingsEnabled(): boolean {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return bindingsEnabled;
return instructionState.bindingsEnabled;
}
@ -91,7 +209,7 @@ export function getBindingsEnabled(): boolean {
* @codeGenApi
*/
export function ɵɵenableBindings(): void {
bindingsEnabled = true;
instructionState.bindingsEnabled = true;
}
/**
@ -114,22 +232,13 @@ export function ɵɵenableBindings(): void {
* @codeGenApi
*/
export function ɵɵdisableBindings(): void {
bindingsEnabled = false;
instructionState.bindingsEnabled = false;
}
export function getLView(): LView {
return lView;
return instructionState.lView;
}
/**
* Used as the starting directive id value.
*
* All subsequent directives are incremented from this value onwards.
* The reason why this value is `1` instead of `0` is because the `0`
* value is reserved for the template.
*/
let activeDirectiveId = 0;
/**
* Flags used for an active element during change detection.
*
@ -150,14 +259,14 @@ export const enum ActiveElementFlags {
* Determines whether or not a flag is currently set for the active element.
*/
export function hasActiveElementFlag(flag: ActiveElementFlags) {
return (_selectedIndex & flag) === flag;
return (instructionState.selectedIndex & flag) === flag;
}
/**
* Sets a flag is for the active element.
*/
export function setActiveElementFlag(flag: ActiveElementFlags) {
_selectedIndex |= flag;
instructionState.selectedIndex |= flag;
}
/**
@ -173,16 +282,15 @@ export function setActiveHostElement(elementIndex: number | null = null) {
executeElementExitFn();
}
setSelectedIndex(elementIndex === null ? -1 : elementIndex);
activeDirectiveId = 0;
instructionState.activeDirectiveId = 0;
}
}
let _elementExitFn: Function|null = null;
export function executeElementExitFn() {
_elementExitFn !();
instructionState.elementExitFn !();
// TODO (matsko|misko): remove this unassignment once the state management of
// global variables are better managed.
_selectedIndex &= ~ActiveElementFlags.RunExitFn;
instructionState.selectedIndex &= ~ActiveElementFlags.RunExitFn;
}
/**
@ -198,9 +306,13 @@ export function executeElementExitFn() {
*
* @param fn
*/
export function setElementExitFn(fn: Function): void {
export function setElementExitFn(fn: () => void): void {
setActiveElementFlag(ActiveElementFlags.RunExitFn);
_elementExitFn = fn;
if (instructionState.elementExitFn == null) {
instructionState.elementExitFn = fn;
}
ngDevMode &&
assertEqual(instructionState.elementExitFn, fn, 'Expecting to always get the same function');
}
/**
@ -219,7 +331,7 @@ export function setElementExitFn(fn: Function): void {
* different set of directives).
*/
export function getActiveDirectiveId() {
return activeDirectiveId;
return instructionState.activeDirectiveId;
}
/**
@ -248,7 +360,7 @@ export function incrementActiveDirectiveId() {
// directive uniqueId is not set anywhere--it is just incremented between
// each hostBindings call and is useful for helping instruction code
// uniquely determine which directive is currently active when executed.
activeDirectiveId += 1;
instructionState.activeDirectiveId += 1;
}
/**
@ -263,113 +375,67 @@ export function incrementActiveDirectiveId() {
* @codeGenApi
*/
export function ɵɵrestoreView(viewToRestore: OpaqueViewState) {
contextLView = viewToRestore as any as LView;
instructionState.contextLView = viewToRestore as any as LView;
}
/** Used to set the parent property when nodes are created and track query results. */
let previousOrParentTNode: TNode;
export function getPreviousOrParentTNode(): TNode {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return previousOrParentTNode;
return instructionState.previousOrParentTNode;
}
export function setPreviousOrParentTNode(tNode: TNode, _isParent: boolean) {
previousOrParentTNode = tNode;
isParent = _isParent;
instructionState.previousOrParentTNode = tNode;
instructionState.isParent = _isParent;
}
export function setTNodeAndViewData(tNode: TNode, view: LView) {
ngDevMode && assertLViewOrUndefined(view);
previousOrParentTNode = tNode;
lView = view;
instructionState.previousOrParentTNode = tNode;
instructionState.lView = view;
}
/**
* If `isParent` is:
* - `true`: then `previousOrParentTNode` points to a parent node.
* - `false`: then `previousOrParentTNode` points to previous node (sibling).
*/
let isParent: boolean;
export function getIsParent(): boolean {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return isParent;
return instructionState.isParent;
}
export function setIsNotParent(): void {
isParent = false;
instructionState.isParent = false;
}
export function setIsParent(): void {
isParent = true;
instructionState.isParent = true;
}
/**
* State of the current view being processed.
*
* An array of nodes (text, element, container, etc), pipes, their bindings, and
* any local variables that need to be stored between invocations.
*/
let lView: LView;
/**
* The last viewData retrieved by nextContext().
* Allows building nextContext() and reference() calls.
*
* e.g. const inner = x().$implicit; const outer = x().$implicit;
*/
let contextLView: LView = null !;
export function getContextLView(): LView {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return contextLView;
return instructionState.contextLView;
}
/**
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
*
* Necessary to support ChangeDetectorRef.checkNoChanges().
*/
let checkNoChangesMode = false;
export function getCheckNoChangesMode(): boolean {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return checkNoChangesMode;
return instructionState.checkNoChangesMode;
}
export function setCheckNoChangesMode(mode: boolean): void {
checkNoChangesMode = mode;
instructionState.checkNoChangesMode = mode;
}
/**
* The root index from which pure function instructions should calculate their binding
* indices. In component views, this is TView.bindingStartIndex. In a host binding
* context, this is the TView.expandoStartIndex + any dirs/hostVars before the given dir.
*/
let bindingRootIndex: number = -1;
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
export function getBindingRoot() {
return bindingRootIndex;
return instructionState.bindingRootIndex;
}
export function setBindingRoot(value: number) {
bindingRootIndex = value;
instructionState.bindingRootIndex = value;
}
/**
* Current index of a View or Content Query which needs to be processed next.
* We iterate over the list of Queries and increment current query index at every step.
*/
let currentQueryIndex: number = 0;
export function getCurrentQueryIndex(): number {
// top level variables should not be exported for performance reasons (PERF_NOTES.md)
return currentQueryIndex;
return instructionState.currentQueryIndex;
}
export function setCurrentQueryIndex(value: number): void {
currentQueryIndex = value;
instructionState.currentQueryIndex = value;
}
/**
@ -390,18 +456,18 @@ export function selectView(newView: LView, hostTNode: TElementNode | TViewNode |
}
ngDevMode && assertLViewOrUndefined(newView);
const oldView = lView;
const oldView = instructionState.lView;
previousOrParentTNode = hostTNode !;
isParent = true;
instructionState.previousOrParentTNode = hostTNode !;
instructionState.isParent = true;
lView = contextLView = newView;
instructionState.lView = instructionState.contextLView = newView;
return oldView;
}
export function nextContextImpl<T = any>(level: number = 1): T {
contextLView = walkUpViews(level, contextLView !);
return contextLView[CONTEXT] as T;
instructionState.contextLView = walkUpViews(level, instructionState.contextLView !);
return instructionState.contextLView[CONTEXT] as T;
}
function walkUpViews(nestingLevel: number, currentView: LView): LView {
@ -419,16 +485,13 @@ function walkUpViews(nestingLevel: number, currentView: LView): LView {
* Resets the application state.
*/
export function resetComponentState() {
isParent = false;
previousOrParentTNode = null !;
elementDepthCount = 0;
bindingsEnabled = true;
instructionState.isParent = false;
instructionState.previousOrParentTNode = null !;
instructionState.elementDepthCount = 0;
instructionState.bindingsEnabled = true;
setCurrentStyleSanitizer(null);
}
/* tslint:disable */
let _selectedIndex = -1 << ActiveElementFlags.Size;
/**
* Gets the most recent index passed to {@link select}
*
@ -436,7 +499,7 @@ let _selectedIndex = -1 << ActiveElementFlags.Size;
* current `LView` to act on.
*/
export function getSelectedIndex() {
return _selectedIndex >> ActiveElementFlags.Size;
return instructionState.selectedIndex >> ActiveElementFlags.Size;
}
/**
@ -449,19 +512,17 @@ export function getSelectedIndex() {
* run if and when the provided `index` value is different from the current selected index value.)
*/
export function setSelectedIndex(index: number) {
_selectedIndex = index << ActiveElementFlags.Size;
instructionState.selectedIndex = index << ActiveElementFlags.Size;
}
let _currentNamespace: string|null = null;
/**
* Sets the namespace used to create elements to `'http://www.w3.org/2000/svg'` in global state.
*
* @codeGenApi
*/
export function ɵɵnamespaceSVG() {
_currentNamespace = 'http://www.w3.org/2000/svg';
instructionState.currentNamespace = 'http://www.w3.org/2000/svg';
}
/**
@ -470,7 +531,7 @@ export function ɵɵnamespaceSVG() {
* @codeGenApi
*/
export function ɵɵnamespaceMathML() {
_currentNamespace = 'http://www.w3.org/1998/MathML/';
instructionState.currentNamespace = 'http://www.w3.org/1998/MathML/';
}
/**
@ -488,16 +549,15 @@ export function ɵɵnamespaceHTML() {
* `createElement` rather than `createElementNS`.
*/
export function namespaceHTMLInternal() {
_currentNamespace = null;
instructionState.currentNamespace = null;
}
export function getNamespace(): string|null {
return _currentNamespace;
return instructionState.currentNamespace;
}
let _currentSanitizer: StyleSanitizeFn|null;
export function setCurrentStyleSanitizer(sanitizer: StyleSanitizeFn | null) {
_currentSanitizer = sanitizer;
instructionState.currentSanitizer = sanitizer;
}
export function resetCurrentStyleSanitizer() {
@ -505,5 +565,5 @@ export function resetCurrentStyleSanitizer() {
}
export function getCurrentStyleSanitizer() {
return _currentSanitizer;
return instructionState.currentSanitizer;
}

View File

@ -140,21 +140,12 @@
{
"name": "__window"
},
{
"name": "_currentNamespace"
},
{
"name": "_elementExitFn"
},
{
"name": "_global"
},
{
"name": "_renderCompCount"
},
{
"name": "_selectedIndex"
},
{
"name": "addComponentLogic"
},
@ -188,9 +179,6 @@
{
"name": "callHooks"
},
{
"name": "checkNoChangesMode"
},
{
"name": "concatString"
},
@ -431,6 +419,9 @@
{
"name": "instantiateRootComponent"
},
{
"name": "instructionState"
},
{
"name": "invertObject"
},

View File

@ -125,18 +125,12 @@
{
"name": "__window"
},
{
"name": "_elementExitFn"
},
{
"name": "_global"
},
{
"name": "_renderCompCount"
},
{
"name": "_selectedIndex"
},
{
"name": "addToViewTree"
},
@ -158,9 +152,6 @@
{
"name": "callHooks"
},
{
"name": "checkNoChangesMode"
},
{
"name": "createLView"
},
@ -332,6 +323,9 @@
{
"name": "instantiateRootComponent"
},
{
"name": "instructionState"
},
{
"name": "invertObject"
},

View File

@ -326,33 +326,21 @@
{
"name": "_currentInjector"
},
{
"name": "_currentNamespace"
},
{
"name": "_devMode"
},
{
"name": "_elementExitFn"
},
{
"name": "_global"
},
{
"name": "_renderCompCount"
},
{
"name": "_selectedIndex"
},
{
"name": "_state"
},
{
"name": "_symbolIterator"
},
{
"name": "activeDirectiveId"
},
{
"name": "addBindingIntoContext"
},
@ -446,9 +434,6 @@
{
"name": "checkNoChangesInternal"
},
{
"name": "checkNoChangesMode"
},
{
"name": "cleanUpView"
},
@ -461,9 +446,6 @@
{
"name": "containerInternal"
},
{
"name": "contextLView"
},
{
"name": "createContainerRef"
},
@ -899,6 +881,9 @@
{
"name": "instantiateRootComponent"
},
{
"name": "instructionState"
},
{
"name": "interpolation1"
},