refactor(ivy): delete ɵɵallocHostVars instruction (#34708)

Delete `ɵɵallocHostVars` instruction in favor of using `hostVars` declaration on `DrictiveDef` directly.

PR Close #34708
This commit is contained in:
Miško Hevery
2020-01-09 12:05:40 -08:00
parent d566621fef
commit 4f3e6e0faf
12 changed files with 150 additions and 155 deletions

View File

@ -200,14 +200,14 @@ The above will create the following layout:
The `EXPANDO` section needs additional information for information stored in `TView.expandoInstructions`
| Index | `TView.expandoInstructions` | Meaning
| ----: | ---------------------------: | -------
| 0 | -10 | Negative numbers signify pointers to elements. In this case 10 (`<child>`)
| 1 | 2 | Injector size. Number of values to skip to get to Host Bindings.
| 2 | Child.ɵcmp.hostBindings | The function to call. (Only when `hostVars` is not `0`)
| 3 | Child.ɵcmp.hostVars | Number of host bindings to process. (Only when `hostVars` is not `0`)
| 4 | Tooltip.ɵdir.hostBindings | The function to call. (Only when `hostVars` is not `0`)
| 5 | Tooltip.ɵdir.hostVars | Number of host bindings to process. (Only when `hostVars` is not `0`)
| Index | `TView.expandoInstructions` | Meaning
| ----: | ---------------------------:| -------
| 0 | -10 | Negative numbers signify pointers to elements. In this case 10 (`<child>`)
| 1 | 2 | Injector size. Number of values to skip to get to Host Bindings.
| 2 | Child.ɵcmp.hostBindings | The function to call. (Only when `hostVars` is not `0`)
| 3 | Child.ɵcmp.hostVars | Number of host bindings to process. (Only when `hostVars` is not `0`)
| 4 | Tooltip.ɵdir.hostBindings | The function to call. (Only when `hostVars` is not `0`)
| 5 | Tooltip.ɵdir.hostVars | Number of host bindings to process. (Only when `hostVars` is not `0`)
The reason for this layout is to make the host binding update efficient using this pseudo code:
```typescript
@ -237,16 +237,16 @@ for(var i = 0; i < tView.expandoInstructions.length; i++) {
The above code should execute as:
| Instruction | `bindingRootIndex` | `currentDirectiveIndex` | `currentElementIndex`
| ----------: | -----------------: | ----------------------: | --------------------:
| (initial) | `11` | `-1` | `-1`
| `-10` | `19` | `\* new Child() *\ 19` | `\* <child> *\ 10`
| `2` | `21` | `\* new Child() *\ 19` | `\* <child> *\ 10`
| Instruction | `bindingRootIndex` | `currentDirectiveIndex` | `currentElementIndex`
| ----------: | -----------------: | ----------------------: | --------------------:
| (initial) | `11` | `-1` | `-1`
| `-10` | `19` | `\* new Child() *\ 19` | `\* <child> *\ 10`
| `2` | `21` | `\* new Child() *\ 19` | `\* <child> *\ 10`
| `Child.ɵcmp.hostBindings` | invoke with => | `\* new Child() *\ 19` | `\* <child> *\ 10`
| | `21` | `\* new Tooltip() *\ 20` | `\* <child> *\ 10`
| | `21` | `\* new Tooltip() *\ 20` | `\* <child> *\ 10`
| `Child.ɵcmp.hostVars` | `22` | `\* new Tooltip() *\ 20` | `\* <child> *\ 10`
| `Tooltip.ɵdir.hostBindings` | invoke with => | `\* new Tooltip() *\ 20` | `\* <child> *\ 10`
| | `22` | `21` | `\* <child> *\ 10`
| | `22` | `21` | `\* <child> *\ 10`
| `Tooltip.ɵdir.hostVars` | `22` | `21` | `\* <child> *\ 10`
## `EXPANDO` and Injection

View File

@ -16,8 +16,8 @@ import {assertDataInRange} from '../util/assert';
import {assertComponentType} from './assert';
import {getComponentDef} from './definition';
import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di';
import {registerPostOrderHooks, registerPreOrderHooks} from './hooks';
import {CLEAN_PROMISE, addToViewTree, createLView, createTView, getOrCreateTComponentView, getOrCreateTNode, initNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, markAsComponentHost, refreshView, renderView} from './instructions/shared';
import {registerPostOrderHooks} from './hooks';
import {CLEAN_PROMISE, addHostBindingsToExpandoInstructions, addToViewTree, createLView, createTView, getOrCreateTComponentView, getOrCreateTNode, growHostVarsSpace, initNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, markAsComponentHost, refreshView, renderView} from './instructions/shared';
import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition';
import {TElementNode, TNode, TNodeType} from './interfaces/node';
import {PlayerHandler} from './interfaces/player';
@ -193,11 +193,11 @@ export function createRootComponentView(
* renderComponent() and ViewContainerRef.createComponent().
*/
export function createRootComponent<T>(
componentView: LView, componentDef: ComponentDef<T>, rootView: LView, rootContext: RootContext,
componentView: LView, componentDef: ComponentDef<T>, rootLView: LView, rootContext: RootContext,
hostFeatures: HostFeature[] | null): any {
const tView = rootView[TVIEW];
const tView = rootLView[TVIEW];
// Create directive instance with factory() and store at next index in viewData
const component = instantiateRootComponent(tView, rootView, componentDef);
const component = instantiateRootComponent(tView, rootLView, componentDef);
rootContext.components.push(component);
componentView[CONTEXT] = component;
@ -207,7 +207,7 @@ export function createRootComponent<T>(
// We want to generate an empty QueryList for root content queries for backwards
// compatibility with ViewEngine.
if (componentDef.contentQueries) {
componentDef.contentQueries(RenderFlags.Create, component, rootView.length - 1);
componentDef.contentQueries(RenderFlags.Create, component, rootLView.length - 1);
}
const rootTNode = getPreviousOrParentTNode();
@ -216,12 +216,15 @@ export function createRootComponent<T>(
// part of the `hostAttrs`.
// The check for componentDef.hostBindings is wrong since now some directives may not
// have componentDef.hostBindings but they still need to process hostVars and hostAttrs
if (tView.firstCreatePass && (componentDef.hostBindings || componentDef.hostVars !== 0 ||
componentDef.hostAttrs !== null)) {
if (tView.firstCreatePass && (componentDef.hostBindings || componentDef.hostAttrs !== null)) {
const elementIndex = rootTNode.index - HEADER_OFFSET;
setActiveHostElement(elementIndex);
incrementActiveDirectiveId();
const rootTView = rootLView[TVIEW];
addHostBindingsToExpandoInstructions(rootTView, componentDef);
growHostVarsSpace(rootTView, rootLView, componentDef.hostVars);
const expando = tView.expandoInstructions !;
invokeHostBindingsInCreationMode(
componentDef, expando, component, rootTNode, tView.firstCreatePass);

View File

@ -25,7 +25,6 @@
*
* Jira Issue = FW-1184
*/
export * from './alloc_host_vars';
export * from './attribute';
export * from './attribute_interpolation';
export * from './change_detection';

View File

@ -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 {assertEqual} from '../../util/assert';
import {ComponentDef, DirectiveDef} from '../interfaces/definition';
import {LView, TVIEW, TView} from '../interfaces/view';
import {getCurrentDirectiveDef, getLView} from '../state';
import {NO_CHANGE} from '../tokens';
// TODO(misko-next): delete alloc_host_vars.ts file.
// TODO(misko-next): delete `ɵɵallocHostVars`
/**
* Allocates the necessary amount of slots for host vars.
*
* @param count Amount of vars to be allocated
*
* @codeGenApi
*/
export function ɵɵallocHostVars(count: number): void {
const lView = getLView();
const tView = lView[TVIEW];
if (!tView.firstCreatePass) return;
queueHostBindingForCheck(tView, getCurrentDirectiveDef() !, count);
prefillHostVars(tView, lView, count);
}
/**
* Stores host binding fn and number of host vars so it will be queued for binding refresh during
* CD.
*/
function queueHostBindingForCheck(
tView: TView, def: DirectiveDef<any>| ComponentDef<any>, hostVars: number): void {
ngDevMode &&
assertEqual(tView.firstCreatePass, true, 'Should only be called in first create pass.');
const expando = tView.expandoInstructions !;
const length = expando.length;
// Check whether a given `hostBindings` function already exists in expandoInstructions,
// which can happen in case directive definition was extended from base definition (as a part of
// the `InheritDefinitionFeature` logic). If we found the same `hostBindings` function in the
// list, we just increase the number of host vars associated with that function, but do not add it
// into the list again.
if (length >= 2 && expando[length - 2] === def.hostBindings) {
expando[length - 1] = (expando[length - 1] as number) + hostVars;
} else {
expando.push(def.hostBindings !, hostVars);
}
}
/**
* On the first template pass, we need to reserve space for host binding values
* after directives are matched (so all directives are saved, then bindings).
* Because we are updating the blueprint, we only need to do this once.
*/
function prefillHostVars(tView: TView, lView: LView, totalHostVars: number): void {
ngDevMode &&
assertEqual(tView.firstCreatePass, true, 'Should only be called in first create pass.');
for (let i = 0; i < totalHostVars; i++) {
lView.push(NO_CHANGE);
tView.blueprint.push(NO_CHANGE);
tView.data.push(null);
}
}

View File

@ -11,7 +11,7 @@ import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../me
import {ViewEncapsulation} from '../../metadata/view';
import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
import {Sanitizer} from '../../sanitization/sanitizer';
import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertNotEqual, assertNotSame} from '../../util/assert';
import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertNotEqual, assertNotSame, assertSame} from '../../util/assert';
import {createNamedArrayType} from '../../util/named_array_type';
import {initNgDevMode} from '../../util/ng_dev_mode';
import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
@ -41,7 +41,6 @@ import {getLViewParent} from '../util/view_traversal_utils';
import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';
import {selectIndexInternal} from './advance';
import {ɵɵallocHostVars} from './alloc_host_vars';
import {ɵɵelementHostAttrs} from './element';
import {LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeConstructor, TNodeInitialInputs, TNodeLocalNames, TViewComponents, TViewConstructor, attachLContainerDebug, attachLViewDebug, cloneToLViewFromTViewBlueprint, cloneToTViewData} from './lview_debug';
@ -53,8 +52,14 @@ import {LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeConstructor, TNod
*/
const _CLEAN_PROMISE = (() => Promise.resolve(null))();
/** Sets the host bindings for the current view. */
export function setHostBindings(tView: TView, lView: LView): void {
/**
* Process the `TVIew.expandoInstructions`. (Execute the `hostBindings`.)
*
* @param tView `TView` containing the `expandoInstructions`
* @param lView `LView` associated with the `TView`
*/
export function setHostBindingsByExecutingExpandoInstructions(tView: TView, lView: LView): void {
ngDevMode && assertSame(tView, lView[TVIEW], '`LView` is not associated with the `TView`!');
const selectedIndex = getSelectedIndex();
try {
const expandoInstructions = tView.expandoInstructions;
@ -63,13 +68,25 @@ export function setHostBindings(tView: TView, lView: LView): void {
setBindingRoot(bindingRootIndex);
let currentDirectiveIndex = -1;
let currentElementIndex = -1;
// TODO(misko): PERF It is possible to get here with `TVIew.expandoInstructions` containing no
// functions to execute. This is wasteful as there is no work to be done, but we still need
// to iterate over the instructions.
// In example of this is in this test: `host_binding_spec.ts`
// `fit('should not cause problems if detectChanges is called when a property updates', ...`
// In the above test we get here with expando [0, 0, 1] which requires a lot of processing but
// there is no function to execute.
for (let i = 0; i < expandoInstructions.length; i++) {
const instruction = expandoInstructions[i];
if (typeof instruction === 'number') {
if (instruction <= 0) {
// Negative numbers mean that we are starting new EXPANDO block and need to update
// the current element and directive index.
currentElementIndex = -instruction;
// Important: In JS `-x` and `0-x` is not the same! If `x===0` then `-x` will produce
// `-0` which requires non standard math arithmetic and it can prevent VM optimizations.
// `0-0` will always produce `0` and will not cause a potential deoptimization in VM.
// TODO(misko): PERF This should be refactored to use `~instruction` as that does not
// suffer from `-0` and it is faster/more compact.
currentElementIndex = 0 - instruction;
setActiveHostElement(currentElementIndex);
// Injector block and providers are taken into account.
@ -97,9 +114,15 @@ export function setHostBindings(tView: TView, lView: LView): void {
incrementActiveDirectiveId();
setBindingIndex(bindingRootIndex);
const hostCtx = unwrapRNode(lView[currentDirectiveIndex]);
const hostCtx = lView[currentDirectiveIndex];
instruction(RenderFlags.Update, hostCtx, currentElementIndex);
}
// TODO(misko): PERF Relying on incrementing the `currentDirectiveIndex` here is
// sub-optimal. The implications are that if we have a lot of directives but none of them
// have host bindings we nevertheless need to iterate over the expando instructions to
// update the counter. It would be much better if we could encode the
// `currentDirectiveIndex` into the `expandoInstruction` array so that we only need to
// iterate over those directives which actually have `hostBindings`.
currentDirectiveIndex++;
}
}
@ -440,7 +463,7 @@ export function refreshView<T>(
}
}
setHostBindings(tView, lView);
setHostBindingsByExecutingExpandoInstructions(tView, lView);
// Refresh child component views.
const components = tView.components;
@ -1099,6 +1122,7 @@ export function resolveDirectives(
let hasDirectives = false;
if (directives !== null) {
let totalDirectiveHostVars = 0;
hasDirectives = true;
initNodeFlags(tNode, tView.data.length, directives.length);
// When the same token is provided by several directives on the same node, some rules apply in
@ -1108,14 +1132,14 @@ export function resolveDirectives(
// So to match these rules, the order in which providers are added in the arrays is very
// important.
for (let i = 0; i < directives.length; i++) {
const def = directives[i] as DirectiveDef<any>;
const def = directives[i];
if (def.providersResolver) def.providersResolver(def);
}
generateExpandoInstructionBlock(tView, tNode, directives.length);
let preOrderHooksFound = false;
let preOrderCheckHooksFound = false;
for (let i = 0; i < directives.length; i++) {
const def = directives[i] as DirectiveDef<any>;
const def = directives[i];
baseResolveDirective(tView, lView, def);
@ -1140,14 +1164,66 @@ export function resolveDirectives(
])).push(tNode.index - HEADER_OFFSET);
preOrderCheckHooksFound = true;
}
addHostBindingsToExpandoInstructions(tView, def);
totalDirectiveHostVars += def.hostVars;
}
initializeInputAndOutputAliases(tView, tNode);
growHostVarsSpace(tView, lView, totalDirectiveHostVars);
}
if (exportsMap) cacheMatchingLocalNames(tNode, localRefs, exportsMap);
return hasDirectives;
}
/**
* Add `hostBindings` to the `TView.expandoInstructions`.
*
* @param tView `TView` to which the `hostBindings` should be added.
* @param def `ComponentDef`/`DirectiveDef`, which contains the `hostVars`/`hostBindings` to add.
*/
export function addHostBindingsToExpandoInstructions(
tView: TView, def: ComponentDef<any>| DirectiveDef<any>): void {
ngDevMode && assertFirstCreatePass(tView);
const expando = tView.expandoInstructions !;
// TODO(misko): PERF we are adding `hostBindings` even if there is nothing to add! This is
// suboptimal for performance. See `currentDirectiveIndex` comment in
// `setHostBindingsByExecutingExpandoInstructions` for details.
// TODO(misko): PERF `def.hostBindings` may be null,
// but we still need to push null to the array as a placeholder
// to ensure the directive counter is incremented (so host
// binding functions always line up with the corrective directive).
// This is suboptimal for performance. See `currentDirectiveIndex`
// comment in `setHostBindingsByExecutingExpandoInstructions` for more
// details. expando.push(def.hostBindings);
expando.push(def.hostBindings);
const hostVars = def.hostVars;
if (hostVars !== 0) {
expando.push(def.hostVars);
}
}
/**
* Grow the `LView`, blueprint and `TView.data` to accommodate the `hostBindings`.
*
* To support locality we don't know ahead of time how many `hostVars` of the containing directives
* we need to allocate. For this reason we allow growing these data structures as we discover more
* directives to accommodate them.
*
* @param tView `TView` which needs to be grown.
* @param lView `LView` which needs to be grown.
* @param count Size by which we need to grow the data structures.
*/
export function growHostVarsSpace(tView: TView, lView: LView, count: number) {
ngDevMode && assertFirstCreatePass(tView);
ngDevMode && assertSame(tView, lView[TVIEW], '`LView` must be associated with `TView`!');
for (let i = 0; i < count; i++) {
lView.push(NO_CHANGE);
tView.blueprint.push(NO_CHANGE);
tView.data.push(null);
}
}
/**
* Instantiate all the directives that were previously resolved on the current node.
*/
@ -1220,9 +1296,6 @@ export function invokeHostBindingsInCreationMode(
// TODO(misko-next): This is a temporary work around for the fact that we moved the information
// from instruction to declaration. The workaround is to just call the instruction as if it was
// part of the `hostAttrs`.
if (def.hostVars !== 0) {
ɵɵallocHostVars(def.hostVars);
}
if (def.hostAttrs !== null) {
ɵɵelementHostAttrs(def.hostAttrs);
}
@ -1230,13 +1303,6 @@ export function invokeHostBindingsInCreationMode(
def.hostBindings !(RenderFlags.Create, directive, elementIndex);
}
setCurrentDirectiveDef(null);
// `hostBindings` function may or may not contain `allocHostVars` call
// (e.g. it may not if it only contains host listeners), so we need to check whether
// `expandoInstructions` has changed and if not - we still push `hostBindings` to
// expando block, to make sure we execute it for DI cycle
if (previousExpandoLength === expando.length && firstCreatePass) {
expando.push(def.hostBindings);
}
}
/**

View File

@ -525,6 +525,8 @@ export interface TView {
*
* See VIEW_DATA.md for more information.
*/
// TODO(misko): `expandoInstructions` should be renamed to `hostBindingsInstructions` since they
// keep track of `hostBindings` which need to be executed.
expandoInstructions: ExpandoInstructions|null;
/**