From 8b26447c4f5ceeb6a6d1e3d9bb1231a096ab0240 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 19 Jul 2019 15:42:08 +0200 Subject: [PATCH] perf(ivy): avoid using array splice (#31656) PR Close #31656 --- .../core/src/render3/node_manipulation.ts | 8 +++---- .../src/render3/view_engine_compatibility.ts | 14 +++++++------ packages/core/src/util/array_utils.ts | 18 ++++++++++++++++ packages/core/src/view/view_attach.ts | 21 ++----------------- .../bundling/todo/bundle.golden_symbols.json | 6 ++++++ 5 files changed, 38 insertions(+), 29 deletions(-) diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 98fb9efe8c..d1f710784c 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -7,8 +7,8 @@ */ import {ViewEncapsulation} from '../metadata/view'; +import {addToArray, removeFromArray} from '../util/array_utils'; import {assertDefined, assertDomNode} from '../util/assert'; - import {assertLContainer, assertLView} from './assert'; import {attachPatchData} from './context_discovery'; import {CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container'; @@ -209,7 +209,7 @@ export function insertView(lView: LView, lContainer: LContainer, index: number) } if (index < containerLength - CONTAINER_HEADER_OFFSET) { lView[NEXT] = lContainer[indexInContainer]; - lContainer.splice(CONTAINER_HEADER_OFFSET + index, 0, lView); + addToArray(lContainer, CONTAINER_HEADER_OFFSET + index, lView); } else { lContainer.push(lView); lView[NEXT] = null; @@ -260,7 +260,7 @@ function detachMovedView(declarationContainer: LContainer, lView: LView) { /** * Detaches a view from a container. * - * This method splices the view from the container's array of active views. It also + * This method removes the view from the container's array of active views. It also * removes the view's elements from the DOM. * * @param lContainer The container from which to detach a view @@ -283,7 +283,7 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView|u if (removeIndex > 0) { lContainer[indexInContainer - 1][NEXT] = viewToDetach[NEXT] as LView; } - const removedLView = lContainer.splice(CONTAINER_HEADER_OFFSET + removeIndex, 1)[0]; + const removedLView = removeFromArray(lContainer, CONTAINER_HEADER_OFFSET + removeIndex); addRemoveViewFromContainer(viewToDetach, false); // notify query that a view has been removed diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index 6d1837dc1d..b35ef17216 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -15,8 +15,8 @@ import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref'; import {ViewContainerRef as ViewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, ViewRef as viewEngine_ViewRef} from '../linker/view_ref'; import {Renderer2} from '../render/api'; +import {addToArray, removeFromArray} from '../util/array_utils'; import {assertDefined, assertGreaterThan, assertLessThan} from '../util/assert'; - import {assertLContainer} from './assert'; import {NodeInjector, getParentInjectorLocation} from './di'; import {addToViewTree, createEmbeddedViewAndNode, createLContainer, renderEmbeddedTemplate} from './instructions/shared'; @@ -201,8 +201,8 @@ export function createContainerRef( } clear(): void { - while (this.length) { - this.remove(0); + while (this.length > 0) { + this.remove(this.length - 1); } } @@ -259,7 +259,7 @@ export function createContainerRef( addRemoveViewFromContainer(lView, true, beforeNode); (viewRef as ViewRef).attachToViewContainerRef(this); - this._lContainer[VIEW_REFS] !.splice(adjustedIdx, 0, viewRef); + addToArray(this._lContainer[VIEW_REFS] !, adjustedIdx, viewRef); return viewRef; } @@ -284,14 +284,16 @@ export function createContainerRef( this.allocateContainerIfNeeded(); const adjustedIdx = this._adjustIndex(index, -1); removeView(this._lContainer, adjustedIdx); - this._lContainer[VIEW_REFS] !.splice(adjustedIdx, 1); + removeFromArray(this._lContainer[VIEW_REFS] !, adjustedIdx); } detach(index?: number): viewEngine_ViewRef|null { this.allocateContainerIfNeeded(); const adjustedIdx = this._adjustIndex(index, -1); const view = detachView(this._lContainer, adjustedIdx); - const wasDetached = view && this._lContainer[VIEW_REFS] !.splice(adjustedIdx, 1)[0] != null; + + const wasDetached = + view && removeFromArray(this._lContainer[VIEW_REFS] !, adjustedIdx) != null; return wasDetached ? new ViewRef(view !, view ![CONTEXT], -1) : null; } diff --git a/packages/core/src/util/array_utils.ts b/packages/core/src/util/array_utils.ts index 1aa1436a8c..e7acac22b3 100644 --- a/packages/core/src/util/array_utils.ts +++ b/packages/core/src/util/array_utils.ts @@ -43,3 +43,21 @@ export function flatten(list: any[], dst?: any[]): any[] { export function deepForEach(input: (T | any[])[], fn: (value: T) => void): void { input.forEach(value => Array.isArray(value) ? deepForEach(value, fn) : fn(value)); } + +export function addToArray(arr: any[], index: number, value: any): void { + // perf: array.push is faster than array.splice! + if (index >= arr.length) { + arr.push(value); + } else { + arr.splice(index, 0, value); + } +} + +export function removeFromArray(arr: any[], index: number): any { + // perf: array.pop is faster than array.splice! + if (index >= arr.length - 1) { + return arr.pop(); + } else { + return arr.splice(index, 1)[0]; + } +} \ No newline at end of file diff --git a/packages/core/src/view/view_attach.ts b/packages/core/src/view/view_attach.ts index 918ab5dd4b..ea86b9a427 100644 --- a/packages/core/src/view/view_attach.ts +++ b/packages/core/src/view/view_attach.ts @@ -6,8 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ +import {addToArray, removeFromArray} from '../util/array_utils'; import {ElementData, NodeDef, NodeFlags, Services, ViewData, ViewDefinition, ViewState} from './types'; -import {RenderNodeAction, declaredViewContainer, isComponentView, renderNode, visitRootRenderNodes} from './util'; +import {RenderNodeAction, declaredViewContainer, renderNode, visitRootRenderNodes} from './util'; export function attachEmbeddedView( parentView: ViewData, elementData: ElementData, viewIndex: number | undefined | null, @@ -133,21 +134,3 @@ function renderAttachEmbeddedView( export function renderDetachView(view: ViewData) { visitRootRenderNodes(view, RenderNodeAction.RemoveChild, null, null, undefined); } - -function addToArray(arr: any[], index: number, value: any) { - // perf: array.push is faster than array.splice! - if (index >= arr.length) { - arr.push(value); - } else { - arr.splice(index, 0, value); - } -} - -function removeFromArray(arr: any[], index: number) { - // perf: array.pop is faster than array.splice! - if (index >= arr.length - 1) { - arr.pop(); - } else { - arr.splice(index, 1); - } -} diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 9fe5ee1f26..11e3dd5f05 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -443,6 +443,9 @@ { "name": "addRemoveViewFromContainer" }, + { + "name": "addToArray" + }, { "name": "addToViewTree" }, @@ -1205,6 +1208,9 @@ { "name": "registerPreOrderHooks" }, + { + "name": "removeFromArray" + }, { "name": "removeListeners" },