Skip to content

Commit

Permalink
perf(animations): improve algorithm to balance animation namespaces (a…
Browse files Browse the repository at this point in the history
…ngular#45057)

The prior approach would consider all existing namespaces from back to front
to find the one that's the closest ancestor for a given host element. An
expensive `contains` operation was used which needed to traverse all the
way up the document root _for each existing namespace_. This commit implements
an optimization where the closest namespace is found by traversing up from
the host element, avoiding repeated DOM traversal.

Closes angular#45055

PR Close angular#45057
  • Loading branch information
JoostK authored and josmar-crwdstffng committed Apr 8, 2022
1 parent 78c1585 commit bf24487
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 16 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/aio-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"master": {
"uncompressed": {
"runtime": 4343,
"main": 449957,
"main": 450729,
"polyfills": 37297,
"styles": 70379,
"light-theme": 77582,
Expand Down
2 changes: 1 addition & 1 deletion packages/animations/browser/src/private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export {AnimationStyleNormalizer as ɵAnimationStyleNormalizer, NoopAnimationSty
export {WebAnimationsStyleNormalizer as ɵWebAnimationsStyleNormalizer} from './dsl/style_normalization/web_animations_style_normalizer';
export {NoopAnimationDriver as ɵNoopAnimationDriver} from './render/animation_driver';
export {AnimationEngine as ɵAnimationEngine} from './render/animation_engine_next';
export {containsElement as ɵcontainsElement, invokeQuery as ɵinvokeQuery, validateStyleProperty as ɵvalidateStyleProperty} from './render/shared';
export {containsElement as ɵcontainsElement, getParentElement as ɵgetParentElement, invokeQuery as ɵinvokeQuery, validateStyleProperty as ɵvalidateStyleProperty} from './render/shared';
export {WebAnimationsDriver as ɵWebAnimationsDriver} from './render/web_animations/web_animations_driver';
export {WebAnimationsPlayer as ɵWebAnimationsPlayer} from './render/web_animations/web_animations_player';
export {allowPreviousPlayerStylesMerge as ɵallowPreviousPlayerStylesMerge, normalizeKeyframes as ɵnormalizeKeyframes} from './util';
16 changes: 14 additions & 2 deletions packages/animations/browser/src/render/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ let _query: (element: any, selector: string, multi: boolean) => any[] =
(element: any, selector: string, multi: boolean) => {
return [];
};
let _documentElement: unknown|null = null;

export function getParentElement(element: any): unknown|null {
const parent = element.parentNode || element.host; // consider host to support shadow DOM
if (parent === _documentElement) {
return null;
}
return parent;
}

// Define utility methods for browsers and platform-server(domino) where Element
// and utility methods exist.
Expand All @@ -151,12 +160,15 @@ if (_isNode || typeof Element !== 'undefined') {
if (!isBrowser()) {
_contains = (elm1, elm2) => elm1.contains(elm2);
} else {
// Read the document element in an IIFE that's been marked pure to avoid a top-level property
// read that may prevent tree-shaking.
_documentElement = /* @__PURE__ */ (() => document.documentElement)();
_contains = (elm1, elm2) => {
while (elm2 && elm2 !== document.documentElement) {
while (elm2) {
if (elm2 === elm1) {
return true;
}
elm2 = elm2.parentNode || elm2.host; // consider host to support shadow DOM
elm2 = getParentElement(elm2);
}
return false;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,25 +589,51 @@ export class TransitionAnimationEngine {
}

private _balanceNamespaceList(ns: AnimationTransitionNamespace, hostElement: any) {
const limit = this._namespaceList.length - 1;
const namespaceList = this._namespaceList;
const namespacesByHostElement = this.namespacesByHostElement;
const limit = namespaceList.length - 1;
if (limit >= 0) {
let found = false;
for (let i = limit; i >= 0; i--) {
const nextNamespace = this._namespaceList[i];
if (this.driver.containsElement(nextNamespace.hostElement, hostElement)) {
this._namespaceList.splice(i + 1, 0, ns);
found = true;
break;
if (this.driver.getParentElement !== undefined) {
// Fast path for when the driver implements `getParentElement`, which allows us to find the
// closest ancestor with an existing namespace that we can then insert `ns` after, without
// having to inspect all existing namespaces.
let ancestor = this.driver.getParentElement(hostElement);
while (ancestor) {
const ancestorNs = namespacesByHostElement.get(ancestor);
if (ancestorNs) {
// An animation namespace has been registered for this ancestor, so we insert `ns`
// right after it to establish top-down ordering of animation namespaces.
const index = namespaceList.indexOf(ancestorNs);
namespaceList.splice(index + 1, 0, ns);
found = true;
break;
}
ancestor = this.driver.getParentElement(ancestor);
}
} else {
// Slow path for backwards compatibility if the driver does not implement
// `getParentElement`, to be removed once `getParentElement` is a required method.
for (let i = limit; i >= 0; i--) {
const nextNamespace = namespaceList[i];
if (this.driver.containsElement(nextNamespace.hostElement, hostElement)) {
namespaceList.splice(i + 1, 0, ns);
found = true;
break;
}
}
}
if (!found) {
this._namespaceList.splice(0, 0, ns);
// No namespace exists that is an ancestor of `ns`, so `ns` is inserted at the front to
// ensure that any existing descendants are ordered after `ns`, retaining the desired
// top-down ordering.
namespaceList.unshift(ns);
}
} else {
this._namespaceList.push(ns);
namespaceList.push(ns);
}

this.namespacesByHostElement.set(hostElement, ns);
namespacesByHostElement.set(hostElement, ns);
return ns;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {AnimationPlayer, ɵStyleDataMap} from '@angular/animations';

import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, copyStyles, normalizeKeyframes} from '../../util';
import {AnimationDriver} from '../animation_driver';
import {containsElement, invokeQuery, isBrowser, validateStyleProperty} from '../shared';
import {containsElement, getParentElement, invokeQuery, validateStyleProperty} from '../shared';
import {packageNonAnimatableStyles} from '../special_cased_styles';

import {WebAnimationsPlayer} from './web_animations_player';
Expand All @@ -28,6 +28,10 @@ export class WebAnimationsDriver implements AnimationDriver {
return containsElement(elm1, elm2);
}

getParentElement(element: unknown): unknown {
return getParentElement(element);
}

query(element: any, selector: string, multi: boolean): any[] {
return invokeQuery(element, selector, multi);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AnimationPlayer, AUTO_STYLE, NoopAnimationPlayer, ɵStyleDataMap} from '@angular/animations';
import {AnimationDriver, ɵallowPreviousPlayerStylesMerge as allowPreviousPlayerStylesMerge, ɵcontainsElement as containsElement, ɵinvokeQuery as invokeQuery, ɵnormalizeKeyframes as normalizeKeyframes, ɵvalidateStyleProperty as validateStyleProperty} from '@angular/animations/browser';
import {AnimationDriver, ɵallowPreviousPlayerStylesMerge as allowPreviousPlayerStylesMerge, ɵcontainsElement as containsElement, ɵgetParentElement as getParentElement, ɵinvokeQuery as invokeQuery, ɵnormalizeKeyframes as normalizeKeyframes, ɵvalidateStyleProperty as validateStyleProperty,} from '@angular/animations/browser';

/**
* @publicApi
Expand All @@ -26,6 +26,10 @@ export class MockAnimationDriver implements AnimationDriver {
return containsElement(elm1, elm2);
}

getParentElement(element: unknown): unknown {
return getParentElement(element);
}

query(element: any, selector: string, multi: boolean): any[] {
return invokeQuery(element, selector, multi);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,9 @@
{
"name": "_currentInjector"
},
{
"name": "_documentElement"
},
{
"name": "_enable_super_gross_mode_that_will_cause_bad_things"
},
Expand Down Expand Up @@ -911,6 +914,9 @@
{
"name": "getOwnDefinition"
},
{
"name": "getParentElement"
},
{
"name": "getParentInjectorIndex"
},
Expand Down

0 comments on commit bf24487

Please sign in to comment.