From bf244874a35b5148aa2e60c065d5abfe0fc262e0 Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 11 Feb 2022 21:46:55 +0100 Subject: [PATCH] perf(animations): improve algorithm to balance animation namespaces (#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 #45055 PR Close #45057 --- goldens/size-tracking/aio-payloads.json | 2 +- .../animations/browser/src/private_export.ts | 2 +- .../animations/browser/src/render/shared.ts | 16 ++++++- .../src/render/transition_animation_engine.ts | 46 +++++++++++++++---- .../web_animations/web_animations_driver.ts | 6 ++- .../testing/src/mock_animation_driver.ts | 6 ++- .../animations/bundle.golden_symbols.json | 6 +++ 7 files changed, 68 insertions(+), 16 deletions(-) diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index c09bc4aa56ec0..e63522b60d88a 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -15,7 +15,7 @@ "master": { "uncompressed": { "runtime": 4343, - "main": 449957, + "main": 450729, "polyfills": 37297, "styles": 70379, "light-theme": 77582, diff --git a/packages/animations/browser/src/private_export.ts b/packages/animations/browser/src/private_export.ts index 533ac3b6bee20..5f1bb28806f98 100644 --- a/packages/animations/browser/src/private_export.ts +++ b/packages/animations/browser/src/private_export.ts @@ -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'; diff --git a/packages/animations/browser/src/render/shared.ts b/packages/animations/browser/src/render/shared.ts index 9562af823969a..b4bf952648a26 100644 --- a/packages/animations/browser/src/render/shared.ts +++ b/packages/animations/browser/src/render/shared.ts @@ -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. @@ -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; }; diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 232c0f63f58c7..f11bdb28c744a 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -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; } diff --git a/packages/animations/browser/src/render/web_animations/web_animations_driver.ts b/packages/animations/browser/src/render/web_animations/web_animations_driver.ts index 5811309f5e861..e67be7d2b9355 100644 --- a/packages/animations/browser/src/render/web_animations/web_animations_driver.ts +++ b/packages/animations/browser/src/render/web_animations/web_animations_driver.ts @@ -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'; @@ -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); } diff --git a/packages/animations/browser/testing/src/mock_animation_driver.ts b/packages/animations/browser/testing/src/mock_animation_driver.ts index 873cf7b8b171f..f42bd30d13093 100644 --- a/packages/animations/browser/testing/src/mock_animation_driver.ts +++ b/packages/animations/browser/testing/src/mock_animation_driver.ts @@ -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 @@ -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); } diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index 606674dd15ead..6dd9a6230613b 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -554,6 +554,9 @@ { "name": "_currentInjector" }, + { + "name": "_documentElement" + }, { "name": "_enable_super_gross_mode_that_will_cause_bad_things" }, @@ -911,6 +914,9 @@ { "name": "getOwnDefinition" }, + { + "name": "getParentElement" + }, { "name": "getParentInjectorIndex" },