Skip to content

Commit 2deac0a

Browse files
matskoalxhub
authored andcommitted
perf(ivy): cache multiple reads to an element's stylingContext (angular#29818)
Because the styling context may be stored in a different location and be apart of a sub array, reading the styling context each time a host binding is evaluated is costly. This patch ensures that the active styling context is cached so that multiple interactions with styling bindings can easily retrieve the styling context efficiently. PR Close angular#29818
1 parent 9147092 commit 2deac0a

File tree

10 files changed

+102
-29
lines changed

10 files changed

+102
-29
lines changed

packages/core/src/debug/debug_node.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {TElementNode, TNode, TNodeFlags, TNodeType} from '../render3/interfaces/
1313
import {StylingIndex} from '../render3/interfaces/styling';
1414
import {LView, NEXT, PARENT, TData, TVIEW, T_HOST} from '../render3/interfaces/view';
1515
import {getProp, getValue, isClassBasedValue} from '../render3/styling/class_and_style_bindings';
16-
import {getStylingContext} from '../render3/styling/util';
16+
import {getStylingContextFromLView} from '../render3/styling/util';
1717
import {getComponent, getContext, getInjectionTokens, getInjector, getListeners, getLocalRefs, isBrowserEvents, loadLContext, loadLContextFromNode} from '../render3/util/discovery_utils';
1818
import {INTERPOLATION_DELIMITER, isPropMetadataString, renderStringify} from '../render3/util/misc_utils';
1919
import {findComponentView} from '../render3/util/view_traversal_utils';
@@ -288,7 +288,7 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
288288
const element = this.nativeElement;
289289
if (element) {
290290
const lContext = loadLContextFromNode(element);
291-
const stylingContext = getStylingContext(lContext.nodeIndex, lContext.lView);
291+
const stylingContext = getStylingContextFromLView(lContext.nodeIndex, lContext.lView);
292292
if (stylingContext) {
293293
for (let i = StylingIndex.SingleStylesStartPosition; i < stylingContext.length;
294294
i += StylingIndex.Size) {
@@ -317,7 +317,7 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
317317
const element = this.nativeElement;
318318
if (element) {
319319
const lContext = loadLContextFromNode(element);
320-
const stylingContext = getStylingContext(lContext.nodeIndex, lContext.lView);
320+
const stylingContext = getStylingContextFromLView(lContext.nodeIndex, lContext.lView);
321321
if (stylingContext) {
322322
for (let i = StylingIndex.SingleStylesStartPosition; i < stylingContext.length;
323323
i += StylingIndex.Size) {

packages/core/src/render3/instructions/element.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@ import {registerPostOrderHooks} from '../hooks';
1313
import {TAttributes, TNodeFlags, TNodeType} from '../interfaces/node';
1414
import {RElement, isProceduralRenderer} from '../interfaces/renderer';
1515
import {SanitizerFn} from '../interfaces/sanitization';
16+
import {StylingContext} from '../interfaces/styling';
1617
import {BINDING_INDEX, QUERIES, RENDERER, TVIEW} from '../interfaces/view';
1718
import {assertNodeType} from '../node_assert';
1819
import {appendChild} from '../node_manipulation';
1920
import {applyOnCreateInstructions} from '../node_util';
2021
import {decreaseElementDepthCount, getActiveDirectiveId, getElementDepthCount, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, getSelectedIndex, increaseElementDepthCount, setIsParent, setPreviousOrParentTNode} from '../state';
2122
import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticContext, patchContextWithStaticAttrs, renderInitialClasses, renderInitialStyles} from '../styling/class_and_style_bindings';
22-
import {getStylingContext, hasClassInput, hasStyleInput} from '../styling/util';
23+
import {getStylingContextFromLView, hasClassInput, hasStyleInput} from '../styling/util';
2324
import {NO_CHANGE} from '../tokens';
2425
import {attrsStylingIndexOf, setUpAttributes} from '../util/attrs_utils';
2526
import {renderStringify} from '../util/misc_utils';
@@ -156,13 +157,15 @@ export function ɵɵelementEnd(): void {
156157
// this is fired at the end of elementEnd because ALL of the stylingBindings code
157158
// (for directives and the template) have now executed which means the styling
158159
// context can be instantiated properly.
160+
let stylingContext: StylingContext|null = null;
159161
if (hasClassInput(previousOrParentTNode)) {
160-
const stylingContext = getStylingContext(previousOrParentTNode.index, lView);
162+
stylingContext = getStylingContextFromLView(previousOrParentTNode.index, lView);
161163
setInputsForProperty(
162164
lView, previousOrParentTNode.inputs !['class'] !, getInitialClassNameValue(stylingContext));
163165
}
164166
if (hasStyleInput(previousOrParentTNode)) {
165-
const stylingContext = getStylingContext(previousOrParentTNode.index, lView);
167+
stylingContext =
168+
stylingContext || getStylingContextFromLView(previousOrParentTNode.index, lView);
166169
setInputsForProperty(
167170
lView, previousOrParentTNode.inputs !['style'] !,
168171
getInitialStyleStringValue(stylingContext));

packages/core/src/render3/instructions/styling.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,17 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88
import {StyleSanitizeFn} from '../../sanitization/style_sanitizer';
9+
import {assertEqual} from '../../util/assert';
910
import {TNode, TNodeType} from '../interfaces/node';
1011
import {PlayerFactory} from '../interfaces/player';
11-
import {FLAGS, HEADER_OFFSET, LViewFlags, RENDERER, RootContextFlags} from '../interfaces/view';
12+
import {FLAGS, HEADER_OFFSET, LView, LViewFlags, RENDERER, RootContextFlags} from '../interfaces/view';
1213
import {getActiveDirectiveId, getActiveDirectiveSuperClassDepth, getLView, getPreviousOrParentTNode, getSelectedIndex} from '../state';
1314
import {getInitialClassNameValue, renderStyling, updateClassProp as updateElementClassProp, updateContextWithBindings, updateStyleProp as updateElementStyleProp, updateStylingMap} from '../styling/class_and_style_bindings';
1415
import {ParamsOf, enqueueHostInstruction, registerHostDirective} from '../styling/host_instructions_queue';
1516
import {BoundPlayerFactory} from '../styling/player_factory';
1617
import {DEFAULT_TEMPLATE_DIRECTIVE_INDEX} from '../styling/shared';
17-
import {allocateOrUpdateDirectiveIntoContext, createEmptyStylingContext, forceClassesAsString, forceStylesAsString, getStylingContext, hasClassInput, hasStyleInput} from '../styling/util';
18+
import {getCachedStylingContext, setCachedStylingContext} from '../styling/state';
19+
import {allocateOrUpdateDirectiveIntoContext, createEmptyStylingContext, forceClassesAsString, forceStylesAsString, getStylingContextFromLView, hasClassInput, hasStyleInput} from '../styling/util';
1820
import {NO_CHANGE} from '../tokens';
1921
import {renderStringify} from '../util/misc_utils';
2022
import {getRootContext} from '../util/view_traversal_utils';
@@ -170,9 +172,9 @@ export function ɵɵelementStyleProp(
170172
index: number, styleIndex: number, value: string | number | String | PlayerFactory | null,
171173
suffix?: string | null, forceOverride?: boolean): void {
172174
const valueToAdd = resolveStylePropValue(value, suffix);
175+
const stylingContext = getStylingContext(index, getLView());
173176
updateElementStyleProp(
174-
getStylingContext(index + HEADER_OFFSET, getLView()), styleIndex, valueToAdd,
175-
DEFAULT_TEMPLATE_DIRECTIVE_INDEX, forceOverride);
177+
stylingContext, styleIndex, valueToAdd, DEFAULT_TEMPLATE_DIRECTIVE_INDEX, forceOverride);
176178
}
177179

178180
/**
@@ -206,9 +208,7 @@ export function ɵɵelementHostStyleProp(
206208
const directiveStylingIndex = getActiveDirectiveStylingIndex();
207209
const hostElementIndex = getSelectedIndex();
208210

209-
const lView = getLView();
210-
const stylingContext = getStylingContext(hostElementIndex + HEADER_OFFSET, lView);
211-
211+
const stylingContext = getStylingContext(hostElementIndex, getLView());
212212
const valueToAdd = resolveStylePropValue(value, suffix);
213213
const args: ParamsOf<typeof updateElementStyleProp> =
214214
[stylingContext, styleIndex, valueToAdd, directiveStylingIndex, forceOverride];
@@ -259,9 +259,9 @@ export function ɵɵelementClassProp(
259259
const input = (value instanceof BoundPlayerFactory) ?
260260
(value as BoundPlayerFactory<boolean|null>) :
261261
booleanOrNull(value);
262+
const stylingContext = getStylingContext(index, getLView());
262263
updateElementClassProp(
263-
getStylingContext(index + HEADER_OFFSET, getLView()), classIndex, input,
264-
DEFAULT_TEMPLATE_DIRECTIVE_INDEX, forceOverride);
264+
stylingContext, classIndex, input, DEFAULT_TEMPLATE_DIRECTIVE_INDEX, forceOverride);
265265
}
266266

267267

@@ -287,9 +287,7 @@ export function ɵɵelementHostClassProp(
287287
classIndex: number, value: boolean | PlayerFactory, forceOverride?: boolean): void {
288288
const directiveStylingIndex = getActiveDirectiveStylingIndex();
289289
const hostElementIndex = getSelectedIndex();
290-
291-
const lView = getLView();
292-
const stylingContext = getStylingContext(hostElementIndex + HEADER_OFFSET, lView);
290+
const stylingContext = getStylingContext(hostElementIndex, getLView());
293291

294292
const input = (value instanceof BoundPlayerFactory) ?
295293
(value as BoundPlayerFactory<boolean|null>) :
@@ -330,8 +328,8 @@ export function ɵɵelementStylingMap(
330328
index: number, classes: {[key: string]: any} | string | NO_CHANGE | null,
331329
styles?: {[styleName: string]: any} | NO_CHANGE | null): void {
332330
const lView = getLView();
331+
const stylingContext = getStylingContext(index, lView);
333332
const tNode = getTNode(index, lView);
334-
const stylingContext = getStylingContext(index + HEADER_OFFSET, lView);
335333

336334
// inputs are only evaluated from a template binding into a directive, therefore,
337335
// there should not be a situation where a directive host bindings function
@@ -384,10 +382,7 @@ export function ɵɵelementHostStylingMap(
384382
styles?: {[styleName: string]: any} | NO_CHANGE | null): void {
385383
const directiveStylingIndex = getActiveDirectiveStylingIndex();
386384
const hostElementIndex = getSelectedIndex();
387-
388-
const lView = getLView();
389-
const stylingContext = getStylingContext(hostElementIndex + HEADER_OFFSET, lView);
390-
385+
const stylingContext = getStylingContext(hostElementIndex, getLView());
391386
const args: ParamsOf<typeof updateStylingMap> =
392387
[stylingContext, classes, styles, directiveStylingIndex];
393388
enqueueHostInstruction(stylingContext, directiveStylingIndex, updateStylingMap, args);
@@ -432,13 +427,22 @@ export function elementStylingApplyInternal(directiveStylingIndex: number, index
432427
// the styling apply code knows not to actually apply the values...
433428
const renderer = tNode.type === TNodeType.Element ? lView[RENDERER] : null;
434429
const isFirstRender = (lView[FLAGS] & LViewFlags.FirstLViewPass) !== 0;
435-
const stylingContext = getStylingContext(index + HEADER_OFFSET, lView);
430+
const stylingContext = getStylingContext(index, lView);
436431
const totalPlayersQueued = renderStyling(
437432
stylingContext, renderer, lView, isFirstRender, null, null, directiveStylingIndex);
438433
if (totalPlayersQueued > 0) {
439434
const rootContext = getRootContext(lView);
440435
scheduleTick(rootContext, RootContextFlags.FlushPlayers);
441436
}
437+
438+
// because select(n) may not run between every instruction, the cached styling
439+
// context may not get cleared between elements. The reason for this is because
440+
// styling bindings (like `[style]` and `[class]`) are not recognized as property
441+
// bindings by default so a select(n) instruction is not generated. To ensure the
442+
// context is loaded correctly for the next element the cache below is pre-emptively
443+
// cleared because there is no code in Angular that applies more styling code after a
444+
// styling flush has occurred. Note that this will be fixed once FW-1254 lands.
445+
setCachedStylingContext(null);
442446
}
443447

444448
export function getActiveDirectiveStylingIndex() {
@@ -450,3 +454,15 @@ export function getActiveDirectiveStylingIndex() {
450454
// sub-classed directive the inheritance depth is taken into account as well.
451455
return getActiveDirectiveId() + getActiveDirectiveSuperClassDepth();
452456
}
457+
458+
function getStylingContext(index: number, lView: LView) {
459+
let context = getCachedStylingContext();
460+
if (!context) {
461+
context = getStylingContextFromLView(index + HEADER_OFFSET, lView);
462+
setCachedStylingContext(context);
463+
} else if (ngDevMode) {
464+
const actualContext = getStylingContextFromLView(index + HEADER_OFFSET, lView);
465+
assertEqual(context, actualContext, 'The cached styling context is invalid');
466+
}
467+
return context;
468+
}

packages/core/src/render3/players.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {getLContext} from './context_discovery';
1111
import {scheduleTick} from './instructions/shared';
1212
import {ComponentInstance, DirectiveInstance, Player} from './interfaces/player';
1313
import {RootContextFlags} from './interfaces/view';
14-
import {addPlayerInternal, getOrCreatePlayerContext, getPlayerContext, getPlayersInternal, getStylingContext, throwInvalidRefError} from './styling/util';
14+
import {addPlayerInternal, getOrCreatePlayerContext, getPlayerContext, getPlayersInternal, getStylingContextFromLView, throwInvalidRefError} from './styling/util';
1515
import {getRootContext} from './util/view_traversal_utils';
1616

1717

@@ -61,7 +61,7 @@ export function getPlayers(ref: ComponentInstance | DirectiveInstance | HTMLElem
6161
return [];
6262
}
6363

64-
const stylingContext = getStylingContext(context.nodeIndex, context.lView);
64+
const stylingContext = getStylingContextFromLView(context.nodeIndex, context.lView);
6565
const playerContext = stylingContext ? getPlayerContext(stylingContext) : null;
6666
return playerContext ? getPlayersInternal(playerContext) : [];
6767
}

packages/core/src/render3/state.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {executeHooks} from './hooks';
1313
import {ComponentDef, DirectiveDef} from './interfaces/definition';
1414
import {TElementNode, TNode, TViewNode} from './interfaces/node';
1515
import {BINDING_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, InitPhaseState, LView, LViewFlags, OpaqueViewState, TVIEW} from './interfaces/view';
16+
import {setCachedStylingContext} from './styling/state';
1617
import {resetPreOrderHookFlags} from './util/view_utils';
1718

1819

@@ -456,6 +457,7 @@ export function leaveView(newView: LView): void {
456457
lView[BINDING_INDEX] = tView.bindingStartIndex;
457458
}
458459
}
460+
setCachedStylingContext(null);
459461
enterView(newView, null);
460462
}
461463

@@ -482,6 +484,10 @@ export function getSelectedIndex() {
482484
*/
483485
export function setSelectedIndex(index: number) {
484486
_selectedIndex = index;
487+
488+
// remove the styling context from the cache
489+
// because we are now on a different element
490+
setCachedStylingContext(null);
485491
}
486492

487493

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
import {StylingContext} from '../interfaces/styling';
9+
10+
let stylingContext: StylingContext|null = null;
11+
12+
/**
13+
* Gets the most recent styling context value.
14+
*
15+
* Note that only one styling context is stored at a given time.
16+
*/
17+
export function getCachedStylingContext() {
18+
return stylingContext;
19+
}
20+
21+
/**
22+
* Sets the most recent styling context value.
23+
*
24+
* Note that only one styling context is stored at a given time.
25+
*
26+
* @param context The styling context value that will be stored
27+
*/
28+
export function setCachedStylingContext(context: StylingContext | null) {
29+
stylingContext = context;
30+
}

packages/core/src/render3/styling/util.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export function allocStylingContext(
124124
* @param index Index of the style allocation. See: `elementStyling`.
125125
* @param viewData The view to search for the styling context
126126
*/
127-
export function getStylingContext(index: number, viewData: LView): StylingContext {
127+
export function getStylingContextFromLView(index: number, viewData: LView): StylingContext {
128128
let storageIndex = index;
129129
let slotValue: LContainer|LView|StylingContext|RElement = viewData[storageIndex];
130130
let wrapper: LContainer|LView|StylingContext = viewData;
@@ -252,7 +252,7 @@ export function getOrCreatePlayerContext(target: {}, context?: LContext | null):
252252
}
253253

254254
const {lView, nodeIndex} = context;
255-
const stylingContext = getStylingContext(nodeIndex, lView);
255+
const stylingContext = getStylingContextFromLView(nodeIndex, lView);
256256
return getPlayerContext(stylingContext) || allocPlayerContext(stylingContext);
257257
}
258258

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@
408408
"name": "getRootView"
409409
},
410410
{
411-
"name": "getStylingContext"
411+
"name": "getStylingContextFromLView"
412412
},
413413
{
414414
"name": "getTNode"
@@ -614,6 +614,9 @@
614614
{
615615
"name": "setBindingRoot"
616616
},
617+
{
618+
"name": "setCachedStylingContext"
619+
},
617620
{
618621
"name": "setClass"
619622
},

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,9 @@
440440
{
441441
"name": "setBindingRoot"
442442
},
443+
{
444+
"name": "setCachedStylingContext"
445+
},
443446
{
444447
"name": "setClass"
445448
},

packages/core/test/bundling/todo/bundle.golden_symbols.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,9 @@
641641
{
642642
"name": "getBindingsEnabled"
643643
},
644+
{
645+
"name": "getCachedStylingContext"
646+
},
644647
{
645648
"name": "getCheckNoChangesMode"
646649
},
@@ -824,6 +827,9 @@
824827
{
825828
"name": "getStylingContext"
826829
},
830+
{
831+
"name": "getStylingContextFromLView"
832+
},
827833
{
828834
"name": "getSymbolIterator"
829835
},
@@ -1178,6 +1184,9 @@
11781184
{
11791185
"name": "setBindingRoot"
11801186
},
1187+
{
1188+
"name": "setCachedStylingContext"
1189+
},
11811190
{
11821191
"name": "setCheckNoChangesMode"
11831192
},
@@ -1265,6 +1274,9 @@
12651274
{
12661275
"name": "stringify"
12671276
},
1277+
{
1278+
"name": "stylingContext"
1279+
},
12681280
{
12691281
"name": "syncViewWithBlueprint"
12701282
},

0 commit comments

Comments
 (0)