From ca0910e59c9d75a4e3719fabe85057146bd8a5fa Mon Sep 17 00:00:00 2001 From: AleksanderBodurri Date: Sat, 14 Nov 2020 17:06:04 -0500 Subject: [PATCH] perf(devtools): prevent directive forest from building twice every time a node is selected in the directive explorer --- .../src/lib/client-event-subscribers.ts | 15 ++++++++++----- .../ng-devtools-backend/src/lib/component-tree.ts | 10 ++++++++-- .../ng-devtools-backend/src/lib/hooks/capture.ts | 2 +- .../ng-devtools-backend/src/lib/hooks/hooks.ts | 15 +++++++++++---- .../src/lib/hooks/identity-tracker.ts | 13 ++++++++++--- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts b/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts index e7bf50098ae..600785f3ee7 100644 --- a/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts +++ b/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts @@ -71,19 +71,21 @@ const getLatestComponentExplorerViewCallback = (messageBus: MessageBus) ) => { // We want to force re-indexing of the component tree. // Pressing the refresh button means the user saw stuck UI. + initializeOrGetDirectiveForestHooks().indexForest(); + if (!query) { messageBus.emit('latestComponentExplorerView', [ { - forest: prepareForestForSerialization(initializeOrGetDirectiveForestHooks().getDirectiveForest()), + forest: prepareForestForSerialization(initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest()), }, ]); return; } messageBus.emit('latestComponentExplorerView', [ { - forest: prepareForestForSerialization(initializeOrGetDirectiveForestHooks().getDirectiveForest()), - properties: getLatestComponentState(query), + forest: prepareForestForSerialization(initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest()), + properties: getLatestComponentState(query, initializeOrGetDirectiveForestHooks().getDirectiveForest()), }, ]); }; @@ -100,7 +102,7 @@ const stopProfilingCallback = (messageBus: MessageBus) => () => { }; const selectedComponentCallback = (position: ElementPosition) => { - const node = queryDirectiveForest(position, initializeOrGetDirectiveForestHooks().getDirectiveForest()); + const node = queryDirectiveForest(position, initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest()); setConsoleReference({ node, position }); }; @@ -109,7 +111,10 @@ const getNestedPropertiesCallback = (messageBus: MessageBus) => ( propPath: string[] ) => { const emitEmpty = () => messageBus.emit('nestedProperties', [position, { props: {} }, propPath]); - const node = queryDirectiveForest(position.element, initializeOrGetDirectiveForestHooks().getDirectiveForest()); + const node = queryDirectiveForest( + position.element, + initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest() + ); if (!node) { return emitEmpty(); } diff --git a/projects/ng-devtools-backend/src/lib/component-tree.ts b/projects/ng-devtools-backend/src/lib/component-tree.ts index 967fd060cbb..0da6c21da85 100644 --- a/projects/ng-devtools-backend/src/lib/component-tree.ts +++ b/projects/ng-devtools-backend/src/lib/component-tree.ts @@ -28,8 +28,14 @@ export interface ComponentTreeNode extends DevToolsNode { - const node = queryDirectiveForest(query.selectedElement, buildDirectiveForest()); +export const getLatestComponentState = ( + query: ComponentExplorerViewQuery, + directiveForest?: ComponentTreeNode[] +): DirectivesProperties | undefined => { + // if a directive forest is passed in we don't have to build the forest again. + directiveForest = directiveForest ?? buildDirectiveForest(); + + const node = queryDirectiveForest(query.selectedElement, directiveForest); if (!node) { return; } diff --git a/projects/ng-devtools-backend/src/lib/hooks/capture.ts b/projects/ng-devtools-backend/src/lib/hooks/capture.ts index 7fc9115cb0c..ce13c6affbc 100644 --- a/projects/ng-devtools-backend/src/lib/hooks/capture.ts +++ b/projects/ng-devtools-backend/src/lib/hooks/capture.ts @@ -197,7 +197,7 @@ const prepareInitialFrame = (source: string, duration: number) => { directives: [], }; const directiveForestHooks = initializeOrGetDirectiveForestHooks(); - const directiveForest = directiveForestHooks.getDirectiveForest(); + const directiveForest = directiveForestHooks.getIndexedDirectiveForest(); const traverse = (node: ComponentTreeNode, children = frame.directives) => { let position: ElementPosition | undefined; if (node.component) { diff --git a/projects/ng-devtools-backend/src/lib/hooks/hooks.ts b/projects/ng-devtools-backend/src/lib/hooks/hooks.ts index 279d27987ae..025d5f65c87 100644 --- a/projects/ng-devtools-backend/src/lib/hooks/hooks.ts +++ b/projects/ng-devtools-backend/src/lib/hooks/hooks.ts @@ -1,3 +1,4 @@ +import { ComponentTreeNode } from './../component-tree'; import { ElementPosition, LifecycleProfile } from 'protocol'; import { componentMetadata, runOutsideAngular } from '../utils'; import { IdentityTracker, IndexedNode } from './identity-tracker'; @@ -109,7 +110,8 @@ export class DirectiveForestHooks { private _undoLifecyclePatch: (() => void)[] = []; private _lastChangeDetection = new Map(); private _tracker = new IdentityTracker(); - private _forest: IndexedNode[] = []; + private _forest: ComponentTreeNode[] = []; + private _indexedForest: IndexedNode[] = []; private _inChangeDetection = false; private _changeDetection$ = new Subject(); @@ -139,7 +141,11 @@ export class DirectiveForestHooks { return result; } - getDirectiveForest(): IndexedNode[] { + getIndexedDirectiveForest(): IndexedNode[] { + return this._indexedForest; + } + + getDirectiveForest(): ComponentTreeNode[] { return this._forest; } @@ -163,8 +169,9 @@ export class DirectiveForestHooks { } indexForest(): void { - const { newNodes, removedNodes, indexedForest } = this._tracker.index(); - this._forest = indexedForest; + const { newNodes, removedNodes, indexedForest, directiveForest } = this._tracker.index(); + this._indexedForest = indexedForest; + this._forest = directiveForest; newNodes.forEach((node) => { this._observeLifecycle(node.directive, node.isComponent); this._observeComponent(node.directive); diff --git a/projects/ng-devtools-backend/src/lib/hooks/identity-tracker.ts b/projects/ng-devtools-backend/src/lib/hooks/identity-tracker.ts index 9d96c755e15..c97e1d23489 100644 --- a/projects/ng-devtools-backend/src/lib/hooks/identity-tracker.ts +++ b/projects/ng-devtools-backend/src/lib/hooks/identity-tracker.ts @@ -1,3 +1,4 @@ +import { ComponentTreeNode } from './../component-tree'; import { ElementPosition, DevToolsNode } from 'protocol'; import { buildDirectiveForest, DirectiveInstanceType, ComponentInstanceType } from '../component-tree'; import { Type } from '@angular/core'; @@ -31,8 +32,14 @@ export class IdentityTracker { return this._currentDirectiveId.has(dir); } - index(): { newNodes: NodeArray; removedNodes: NodeArray; indexedForest: IndexedNode[] } { - const indexedForest = indexForest(buildDirectiveForest()); + index(): { + newNodes: NodeArray; + removedNodes: NodeArray; + indexedForest: IndexedNode[]; + directiveForest: ComponentTreeNode[]; + } { + const directiveForest = buildDirectiveForest(); + const indexedForest = indexForest(directiveForest); const newNodes: NodeArray = []; const removedNodes: NodeArray = []; const allNodes = new Set(); @@ -46,7 +53,7 @@ export class IdentityTracker { // this._currentDirectivePosition.delete(dir); } }); - return { newNodes, removedNodes, indexedForest }; + return { newNodes, removedNodes, indexedForest, directiveForest }; } private _index(