Skip to content

Commit

Permalink
perf(ivy): split hooks processing into init and check phases (#32131)
Browse files Browse the repository at this point in the history
Angular hooks come after 2 flavours:
- init hooks (OnInit, AfterContentInit, AfterViewInit);
- check hooks (OnChanges, DoChanges, AfterContentChecked, AfterViewChecked).

We need to do more processing for init hooks to ensure that those hooks
are run once and only once for a given directive (even in case of errors).
As soon as all init hooks execute to completion we are only left with the
checks to execute.

It turns out that keeping track of the remaining init hooks to execute is
rather expensive (multiple LView flags reads, writes and checks). But we can
observe that non of this tracking is needed as soon as all init hooks are
completed.

This PR takes advantage of the above observations and splits hooks processing
functions into:
- init-specific (slower but less common);
- check-specific (faster and more common).

NOTE: there is code duplication in this PR and it is left like this intentinally:
hand-inlining this perf-critical code makes the view refresh process substentially
faster.

PR Close #32131
  • Loading branch information
pkozlowski-opensource authored and AndrewKushnir committed Aug 21, 2019
1 parent 4d549f6 commit 1062960
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 77 deletions.
2 changes: 1 addition & 1 deletion integration/_payload-limits.json
Expand Up @@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime": 1440,
"main": 13164,
"main": 13411,
"polyfills": 45340
}
}
Expand Down
89 changes: 40 additions & 49 deletions packages/core/src/render3/hooks.ts
Expand Up @@ -6,11 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/

import {assertEqual} from '../util/assert';
import {assertEqual, assertNotEqual} from '../util/assert';

import {DirectiveDef} from './interfaces/definition';
import {TNode} from './interfaces/node';
import {FLAGS, HookData, InitPhaseState, LView, LViewFlags, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TView} from './interfaces/view';
import {getCheckNoChangesMode} from './state';



Expand Down Expand Up @@ -138,70 +139,57 @@ export function registerPostOrderHooks(tView: TView, tNode: TNode): void {
* They are are stored as flags in LView[PREORDER_HOOK_FLAGS].
*/


/**
* Executes necessary hooks at the start of executing a template.
*
* Executes hooks that are to be run during the initialization of a directive such
* as `onChanges`, `onInit`, and `doCheck`.
*
* @param lView The current view
* @param tView Static data for the view containing the hooks to be executed
* @param checkNoChangesMode Whether or not we're in checkNoChanges mode.
* @param @param currentNodeIndex 2 cases depending the the value:
* - undefined: execute hooks only from the saved index until the end of the array (pre-order case,
* when flushing the remaining hooks)
* Executes pre-order check hooks ( OnChanges, DoChanges) given a view where all the init hooks were
* executed once. This is a light version of executeInitAndCheckPreOrderHooks where we can skip read
* / write of the init-hooks related flags.
* @param lView The LView where hooks are defined
* @param hooks Hooks to be run
* @param nodeIndex 3 cases depending on the value:
* - undefined: all hooks from the array should be executed (post-order case)
* - null: execute hooks only from the saved index until the end of the array (pre-order case, when
* flushing the remaining hooks)
* - number: execute hooks only from the saved index until that node index exclusive (pre-order
* case, when executing select(number))
*/
export function executePreOrderHooks(
currentView: LView, tView: TView, checkNoChangesMode: boolean,
currentNodeIndex: number | undefined): void {
if (!checkNoChangesMode) {
executeHooks(
currentView, tView.preOrderHooks, tView.preOrderCheckHooks, checkNoChangesMode,
InitPhaseState.OnInitHooksToBeRun,
currentNodeIndex !== undefined ? currentNodeIndex : null);
}
export function executeCheckHooks(lView: LView, hooks: HookData, nodeIndex?: number | null) {
callHooks(lView, hooks, InitPhaseState.InitPhaseCompleted, nodeIndex);
}

/**
* Executes hooks against the given `LView` based off of whether or not
* This is the first pass.
*
* @param currentView The view instance data to run the hooks against
* @param firstPassHooks An array of hooks to run if we're in the first view pass
* @param checkHooks An Array of hooks to run if we're not in the first view pass.
* @param checkNoChangesMode Whether or not we're in no changes mode.
* @param initPhaseState the current state of the init phase
* @param currentNodeIndex 3 cases depending the the value:
* Executes post-order init and check hooks (one of AfterContentInit, AfterContentChecked,
* AfterViewInit, AfterViewChecked) given a view where there are pending init hooks to be executed.
* @param lView The LView where hooks are defined
* @param hooks Hooks to be run
* @param initPhase A phase for which hooks should be run
* @param nodeIndex 3 cases depending on the value:
* - undefined: all hooks from the array should be executed (post-order case)
* - null: execute hooks only from the saved index until the end of the array (pre-order case, when
* flushing the remaining hooks)
* - number: execute hooks only from the saved index until that node index exclusive (pre-order
* case, when executing select(number))
*/
export function executeHooks(
currentView: LView, firstPassHooks: HookData | null, checkHooks: HookData | null,
checkNoChangesMode: boolean, initPhaseState: InitPhaseState,
currentNodeIndex: number | null | undefined): void {
if (checkNoChangesMode) return;

if (checkHooks !== null || firstPassHooks !== null) {
const hooksToCall = (currentView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhaseState ?
firstPassHooks :
checkHooks;
if (hooksToCall !== null) {
callHooks(currentView, hooksToCall, initPhaseState, currentNodeIndex);
}
export function executeInitAndCheckHooks(
lView: LView, hooks: HookData, initPhase: InitPhaseState, nodeIndex?: number | null) {
ngDevMode && assertNotEqual(
initPhase, InitPhaseState.InitPhaseCompleted,
'Init pre-order hooks should not be called more than once');
if ((lView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhase) {
callHooks(lView, hooks, initPhase, nodeIndex);
}
}

// The init phase state must be always checked here as it may have been recursively updated
let flags = currentView[FLAGS];
if (currentNodeIndex == null && (flags & LViewFlags.InitPhaseStateMask) === initPhaseState &&
initPhaseState !== InitPhaseState.InitPhaseCompleted) {
export function incrementInitPhaseFlags(lView: LView, initPhase: InitPhaseState): void {
ngDevMode &&
assertNotEqual(
initPhase, InitPhaseState.InitPhaseCompleted,
'Init hooks phase should not be incremented after all init hooks have been run.');
let flags = lView[FLAGS];
if ((flags & LViewFlags.InitPhaseStateMask) === initPhase) {
flags &= LViewFlags.IndexWithinInitPhaseReset;
flags += LViewFlags.InitPhaseStateIncrementer;
currentView[FLAGS] = flags;
lView[FLAGS] = flags;
}
}

Expand All @@ -212,7 +200,7 @@ export function executeHooks(
* @param currentView The current view
* @param arr The array in which the hooks are found
* @param initPhaseState the current state of the init phase
* @param currentNodeIndex 3 cases depending the the value:
* @param currentNodeIndex 3 cases depending on the value:
* - undefined: all hooks from the array should be executed (post-order case)
* - null: execute hooks only from the saved index until the end of the array (pre-order case, when
* flushing the remaining hooks)
Expand All @@ -222,6 +210,9 @@ export function executeHooks(
function callHooks(
currentView: LView, arr: HookData, initPhase: InitPhaseState,
currentNodeIndex: number | null | undefined): void {
ngDevMode && assertEqual(
getCheckNoChangesMode(), false,
'Hooks should never be run in the check no changes mode.');
const startIndex = currentNodeIndex !== undefined ?
(currentView[PREORDER_HOOK_FLAGS] & PreOrderHookFlags.IndexOfTheNextPreOrderHookMaskMask) :
0;
Expand Down
21 changes: 18 additions & 3 deletions packages/core/src/render3/instructions/container.ts
Expand Up @@ -8,11 +8,11 @@
import {assertDataInRange, assertEqual} from '../../util/assert';
import {assertHasParent} from '../assert';
import {attachPatchData} from '../context_discovery';
import {executePreOrderHooks, registerPostOrderHooks} from '../hooks';
import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags, registerPostOrderHooks} from '../hooks';
import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
import {ComponentTemplate} from '../interfaces/definition';
import {LocalRefExtractor, TAttributes, TContainerNode, TNode, TNodeType, TViewNode} from '../interfaces/node';
import {BINDING_INDEX, HEADER_OFFSET, LView, RENDERER, TVIEW, T_HOST} from '../interfaces/view';
import {BINDING_INDEX, FLAGS, HEADER_OFFSET, InitPhaseState, LView, LViewFlags, RENDERER, TVIEW, T_HOST} from '../interfaces/view';
import {assertNodeType} from '../node_assert';
import {appendChild, removeView} from '../node_manipulation';
import {getCheckNoChangesMode, getIsParent, getLView, getPreviousOrParentTNode, setIsNotParent, setPreviousOrParentTNode} from '../state';
Expand Down Expand Up @@ -112,7 +112,22 @@ export function ɵɵcontainerRefreshStart(index: number): void {

// We need to execute init hooks here so ngOnInit hooks are called in top level views
// before they are called in embedded views (for backwards compatibility).
executePreOrderHooks(lView, tView, getCheckNoChangesMode(), undefined);
if (!getCheckNoChangesMode()) {
const hooksInitPhaseCompleted =
(lView[FLAGS] & LViewFlags.InitPhaseStateMask) === InitPhaseState.InitPhaseCompleted;
if (hooksInitPhaseCompleted) {
const preOrderCheckHooks = tView.preOrderCheckHooks;
if (preOrderCheckHooks !== null) {
executeCheckHooks(lView, preOrderCheckHooks, null);
}
} else {
const preOrderHooks = tView.preOrderHooks;
if (preOrderHooks !== null) {
executeInitAndCheckHooks(lView, preOrderHooks, InitPhaseState.OnInitHooksToBeRun, null);
}
incrementInitPhaseFlags(lView, InitPhaseState.OnInitHooksToBeRun);
}
}
}

/**
Expand Down
22 changes: 19 additions & 3 deletions packages/core/src/render3/instructions/select.ts
Expand Up @@ -6,11 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/
import {assertDataInRange, assertGreaterThan} from '../../util/assert';
import {executePreOrderHooks} from '../hooks';
import {HEADER_OFFSET, LView, TVIEW} from '../interfaces/view';
import {executeCheckHooks, executeInitAndCheckHooks} from '../hooks';
import {FLAGS, HEADER_OFFSET, InitPhaseState, LView, LViewFlags, TVIEW} from '../interfaces/view';
import {getCheckNoChangesMode, getLView, setSelectedIndex} from '../state';



/**
* Selects an element for later binding instructions.
*
Expand Down Expand Up @@ -41,7 +42,22 @@ export function selectInternal(lView: LView, index: number, checkNoChangesMode:
ngDevMode && assertDataInRange(lView, index + HEADER_OFFSET);

// Flush the initial hooks for elements in the view that have been added up to this point.
executePreOrderHooks(lView, lView[TVIEW], checkNoChangesMode, index);
// PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) {
const hooksInitPhaseCompleted =
(lView[FLAGS] & LViewFlags.InitPhaseStateMask) === InitPhaseState.InitPhaseCompleted;
if (hooksInitPhaseCompleted) {
const preOrderCheckHooks = lView[TVIEW].preOrderCheckHooks;
if (preOrderCheckHooks !== null) {
executeCheckHooks(lView, preOrderCheckHooks, index);
}
} else {
const preOrderHooks = lView[TVIEW].preOrderHooks;
if (preOrderHooks !== null) {
executeInitAndCheckHooks(lView, preOrderHooks, InitPhaseState.OnInitHooksToBeRun, index);
}
}
}

// We must set the selected index *after* running the hooks, because hooks may have side-effects
// that cause other template functions to run, thus updating the selected index, which is global
Expand Down
67 changes: 55 additions & 12 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -17,7 +17,7 @@ import {assertFirstTemplatePass, assertLView} from '../assert';
import {attachPatchData, getComponentViewByInstance} from '../context_discovery';
import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di';
import {throwMultipleComponentError} from '../errors';
import {executeHooks, executePreOrderHooks, registerPreOrderHooks} from '../hooks';
import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags, registerPreOrderHooks} from '../hooks';
import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, FactoryFn, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from '../interfaces/injector';
Expand Down Expand Up @@ -372,7 +372,8 @@ export function renderView<T>(lView: LView, tView: TView, context: T): void {
export function refreshView<T>(
lView: LView, tView: TView, templateFn: ComponentTemplate<{}>| null, context: T) {
ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode');
let oldView = enterView(lView, lView[T_HOST]);
const oldView = enterView(lView, lView[T_HOST]);
const flags = lView[FLAGS];
try {
resetPreOrderHookFlags(lView);

Expand All @@ -385,8 +386,25 @@ export function refreshView<T>(
lView[BINDING_INDEX] = tView.bindingStartIndex;

const checkNoChangesMode = getCheckNoChangesMode();

executePreOrderHooks(lView, tView, checkNoChangesMode, undefined);
const hooksInitPhaseCompleted =
(flags & LViewFlags.InitPhaseStateMask) === InitPhaseState.InitPhaseCompleted;

// execute pre-order hooks (OnInit, OnChanges, DoChanges)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) {
if (hooksInitPhaseCompleted) {
const preOrderCheckHooks = tView.preOrderCheckHooks;
if (preOrderCheckHooks !== null) {
executeCheckHooks(lView, preOrderCheckHooks, null);
}
} else {
const preOrderHooks = tView.preOrderHooks;
if (preOrderHooks !== null) {
executeInitAndCheckHooks(lView, preOrderHooks, InitPhaseState.OnInitHooksToBeRun, null);
}
incrementInitPhaseFlags(lView, InitPhaseState.OnInitHooksToBeRun);
}
}

refreshDynamicEmbeddedViews(lView);

Expand All @@ -395,10 +413,23 @@ export function refreshView<T>(
refreshContentQueries(tView, lView);
}

resetPreOrderHookFlags(lView);
executeHooks(
lView, tView.contentHooks, tView.contentCheckHooks, checkNoChangesMode,
InitPhaseState.AfterContentInitHooksToBeRun, undefined);
// execute content hooks (AfterContentInit, AfterContentChecked)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) {
if (hooksInitPhaseCompleted) {
const contentCheckHooks = tView.contentCheckHooks;
if (contentCheckHooks !== null) {
executeCheckHooks(lView, contentCheckHooks);
}
} else {
const contentHooks = tView.contentHooks;
if (contentHooks !== null) {
executeInitAndCheckHooks(
lView, contentHooks, InitPhaseState.AfterContentInitHooksToBeRun);
}
incrementInitPhaseFlags(lView, InitPhaseState.AfterContentInitHooksToBeRun);
}
}

setHostBindings(tView, lView);

Expand All @@ -413,10 +444,22 @@ export function refreshView<T>(
refreshChildComponents(lView, components);
}

resetPreOrderHookFlags(lView);
executeHooks(
lView, tView.viewHooks, tView.viewCheckHooks, checkNoChangesMode,
InitPhaseState.AfterViewInitHooksToBeRun, undefined);
// execute view hooks (AfterViewInit, AfterViewChecked)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!checkNoChangesMode) {
if (hooksInitPhaseCompleted) {
const viewCheckHooks = tView.viewCheckHooks;
if (viewCheckHooks !== null) {
executeCheckHooks(lView, viewCheckHooks);
}
} else {
const viewHooks = tView.viewHooks;
if (viewHooks !== null) {
executeInitAndCheckHooks(lView, viewHooks, InitPhaseState.AfterViewInitHooksToBeRun);
}
incrementInitPhaseFlags(lView, InitPhaseState.AfterViewInitHooksToBeRun);
}
}

} finally {
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
Expand Down
Expand Up @@ -252,13 +252,13 @@
"name": "enterView"
},
{
"name": "executeContentQueries"
"name": "executeCheckHooks"
},
{
"name": "executeHooks"
"name": "executeContentQueries"
},
{
"name": "executePreOrderHooks"
"name": "executeInitAndCheckHooks"
},
{
"name": "executeTemplate"
Expand Down Expand Up @@ -416,6 +416,9 @@
{
"name": "incrementActiveDirectiveId"
},
{
"name": "incrementInitPhaseFlags"
},
{
"name": "initNodeFlags"
},
Expand Down
Expand Up @@ -210,10 +210,10 @@
"name": "enterView"
},
{
"name": "executeHooks"
"name": "executeCheckHooks"
},
{
"name": "executePreOrderHooks"
"name": "executeInitAndCheckHooks"
},
{
"name": "executeTemplate"
Expand Down Expand Up @@ -323,6 +323,9 @@
{
"name": "incrementActiveDirectiveId"
},
{
"name": "incrementInitPhaseFlags"
},
{
"name": "initNodeFlags"
},
Expand Down

0 comments on commit 1062960

Please sign in to comment.