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 2961bf06c6
commit 2227d471a4
12 changed files with 150 additions and 155 deletions

View File

@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 18496,
"main-es2015": 18271,
"polyfills-es2015": 36808
}
}

View File

@ -201,7 +201,7 @@ 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`)

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;
/**

View File

@ -0,0 +1,13 @@
# Bundle
## `js_expected_symbol_test`
This folder contains tests which assert that most of the code is tree shaken away.
This is asserted by keeping gold files of all symbols which are expected to be retained.
When doing renaming it is often necessary to update the gold files, to do so use this commands:
```
yarn bazel run --config=ivy //packages/core/test/bundling/cyclic_import:symbol_test.accept
yarn bazel run --config=ivy //packages/core/test/bundling/hello_world:symbol_test.accept
yarn bazel run --config=ivy //packages/core/test/bundling/injection:symbol_test.accept
yarn bazel run --config=ivy //packages/core/test/bundling/todo:symbol_test.accept
```

View File

@ -155,6 +155,9 @@
{
"name": "addComponentLogic"
},
{
"name": "addHostBindingsToExpandoInstructions"
},
{
"name": "addItemToStylingMap"
},
@ -314,9 +317,6 @@
{
"name": "getContainerRenderParent"
},
{
"name": "getCurrentDirectiveDef"
},
{
"name": "getDirectiveDef"
},
@ -410,6 +410,9 @@
{
"name": "getTNode"
},
{
"name": "growHostVarsSpace"
},
{
"name": "hasActiveElementFlag"
},
@ -552,10 +555,7 @@
"name": "objectToClassName"
},
{
"name": "prefillHostVars"
},
{
"name": "queueHostBindingForCheck"
"name": "setHostBindingsByExecutingExpandoInstructions"
},
{
"name": "refreshChildComponents"
@ -647,9 +647,6 @@
{
"name": "setDirectiveStylingInput"
},
{
"name": "setHostBindings"
},
{
"name": "setIncludeViewProviders"
},
@ -707,9 +704,6 @@
{
"name": "writeStylingValueDirectly"
},
{
"name": "ɵɵallocHostVars"
},
{
"name": "ɵɵdefineComponent"
},

View File

@ -140,6 +140,9 @@
{
"name": "_renderCompCount"
},
{
"name": "addHostBindingsToExpandoInstructions"
},
{
"name": "addItemToStylingMap"
},
@ -263,9 +266,6 @@
{
"name": "getContainerRenderParent"
},
{
"name": "getCurrentDirectiveDef"
},
{
"name": "getDirectiveDef"
},
@ -350,6 +350,9 @@
{
"name": "getTNode"
},
{
"name": "growHostVarsSpace"
},
{
"name": "hasActiveElementFlag"
},
@ -447,10 +450,7 @@
"name": "objectToClassName"
},
{
"name": "prefillHostVars"
},
{
"name": "queueHostBindingForCheck"
"name": "setHostBindingsByExecutingExpandoInstructions"
},
{
"name": "refreshChildComponents"
@ -524,9 +524,6 @@
{
"name": "setCurrentQueryIndex"
},
{
"name": "setHostBindings"
},
{
"name": "setIncludeViewProviders"
},
@ -572,9 +569,6 @@
{
"name": "writeStylingValueDirectly"
},
{
"name": "ɵɵallocHostVars"
},
{
"name": "ɵɵdefineComponent"
},

View File

@ -344,6 +344,9 @@
{
"name": "addComponentLogic"
},
{
"name": "addHostBindingsToExpandoInstructions"
},
{
"name": "addItemToStylingMap"
},
@ -647,9 +650,6 @@
{
"name": "getContextLView"
},
{
"name": "getCurrentDirectiveDef"
},
{
"name": "getCurrentStyleSanitizer"
},
@ -824,6 +824,9 @@
{
"name": "getValuesCount"
},
{
"name": "growHostVarsSpace"
},
{
"name": "handleError"
},
@ -1083,10 +1086,7 @@
"name": "patchHostStylingFlag"
},
{
"name": "prefillHostVars"
},
{
"name": "queueHostBindingForCheck"
"name": "setHostBindingsByExecutingExpandoInstructions"
},
{
"name": "readPatchedData"
@ -1238,9 +1238,6 @@
{
"name": "setGuardMask"
},
{
"name": "setHostBindings"
},
{
"name": "setIncludeViewProviders"
},
@ -1367,9 +1364,6 @@
{
"name": "ɵɵadvance"
},
{
"name": "ɵɵallocHostVars"
},
{
"name": "ɵɵclassProp"
},

View File

@ -9,7 +9,7 @@
import {RendererType2} from '../../src/render/api';
import {getLContext} from '../../src/render3/context_discovery';
import {AttributeMarker, ɵɵadvance, ɵɵattribute, ɵɵdefineComponent, ɵɵdefineDirective, ɵɵhostProperty, ɵɵproperty} from '../../src/render3/index';
import {ɵɵallocHostVars, ɵɵcontainer, ɵɵcontainerRefreshEnd, ɵɵcontainerRefreshStart, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵembeddedViewEnd, ɵɵembeddedViewStart, ɵɵprojection, ɵɵprojectionDef, ɵɵtemplate, ɵɵtext, ɵɵtextInterpolate} from '../../src/render3/instructions/all';
import {ɵɵcontainer, ɵɵcontainerRefreshEnd, ɵɵcontainerRefreshStart, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵembeddedViewEnd, ɵɵembeddedViewStart, ɵɵprojection, ɵɵprojectionDef, ɵɵtemplate, ɵɵtext, ɵɵtextInterpolate} from '../../src/render3/instructions/all';
import {MONKEY_PATCH_KEY_NAME} from '../../src/render3/interfaces/context';
import {RenderFlags} from '../../src/render3/interfaces/definition';
import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from '../../src/render3/interfaces/renderer';
@ -1112,10 +1112,8 @@ describe('sanitization', () => {
static ɵdir = ɵɵdefineDirective({
type: UnsafeUrlHostBindingDir,
selectors: [['', 'unsafeUrlHostBindingDir', '']],
hostVars: 1,
hostBindings: (rf: RenderFlags, ctx: any) => {
if (rf & RenderFlags.Create) {
ɵɵallocHostVars(1);
}
if (rf & RenderFlags.Update) {
ɵɵhostProperty('cite', ctx.cite, ɵɵsanitizeUrl);
}