From ed7e45d48d0c362a4ae9e6dfce84cb784dcdee42 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 13 Oct 2023 13:25:49 +0200 Subject: [PATCH] refactor(core): move key calculation in list reconciler (#52227) We can speedup items comparision by having access to raw values and delay key calculation in certain conditions. PR Close #52227 --- .../src/render3/instructions/control_flow.ts | 12 ++-- .../core/src/render3/list_reconciliation.ts | 22 ++++--- .../test/render3/list_reconciliation_spec.ts | 60 +++++++++---------- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/packages/core/src/render3/instructions/control_flow.ts b/packages/core/src/render3/instructions/control_flow.ts index 571bac2de6b..75ac6c8ee9f 100644 --- a/packages/core/src/render3/instructions/control_flow.ts +++ b/packages/core/src/render3/instructions/control_flow.ts @@ -153,7 +153,7 @@ export function ɵɵrepeaterCreate( } class LiveCollectionLContainerImpl extends - LiveCollection>, RepeaterContext> { + LiveCollection>, unknown> { /** Property indicating if indexes in the repeater context need to be updated following the live collection changes. Index updates are necessary if and only if views are inserted / removed in @@ -161,16 +161,15 @@ class LiveCollectionLContainerImpl extends */ private needsIndexUpdate = false; constructor( - private lContainer: LContainer, private hostLView: LView, private templateTNode: TNode, - private trackByFn: TrackByFunction) { + private lContainer: LContainer, private hostLView: LView, private templateTNode: TNode) { super(); } override get length(): number { return this.lContainer.length - CONTAINER_HEADER_OFFSET; } - override key(index: number): unknown { - return this.trackByFn(index, this.getLView(index)[CONTEXT].$implicit); + override at(index: number): unknown { + return this.getLView(index)[CONTEXT].$implicit; } override attach(index: number, lView: LView>): void { const dehydratedView = lView[HYDRATION] as DehydratedContainerView; @@ -230,8 +229,7 @@ export function ɵɵrepeater( const lContainer = getLContainer(hostLView, HEADER_OFFSET + containerIndex); const itemTemplateTNode = getExistingTNode(hostTView, containerIndex); - const liveCollection = new LiveCollectionLContainerImpl( - lContainer, hostLView, itemTemplateTNode, metadata.trackByFn); + const liveCollection = new LiveCollectionLContainerImpl(lContainer, hostLView, itemTemplateTNode); reconcile(liveCollection, collection, metadata.trackByFn); // moves in the container might caused context's index to get out of order, re-adjust if needed diff --git a/packages/core/src/render3/list_reconciliation.ts b/packages/core/src/render3/list_reconciliation.ts index 3c1579ede8c..a6cb347c1b9 100644 --- a/packages/core/src/render3/list_reconciliation.ts +++ b/packages/core/src/render3/list_reconciliation.ts @@ -15,7 +15,7 @@ import {TrackByFunction} from '../change_detection'; */ export abstract class LiveCollection { abstract get length(): number; - abstract key(index: number): unknown; + abstract at(index: number): V; abstract attach(index: number, item: T): void; abstract detach(index: number): T; abstract create(index: number, value: V): T; @@ -83,7 +83,8 @@ export function reconcile( while (liveStartIdx <= liveEndIdx && liveStartIdx <= newEndIdx) { // compare from the beginning - const liveStartKey = liveCollection.key(liveStartIdx); + const liveStartValue = liveCollection.at(liveStartIdx); + const liveStartKey = trackByFn(liveStartIdx, liveStartValue); const newStartValue = newCollection[liveStartIdx]; const newStartKey = trackByFn(liveStartIdx, newStartValue); if (Object.is(liveStartKey, newStartKey)) { @@ -94,7 +95,8 @@ export function reconcile( // compare from the end // TODO(perf): do _all_ the matching from the end - const liveEndKey = liveCollection.key(liveEndIdx); + const liveEndValue = liveCollection.at(liveEndIdx); + const liveEndKey = trackByFn(liveEndIdx, liveEndValue); const newEndItem = newCollection[newEndIdx]; const newEndKey = trackByFn(newEndIdx, newEndItem); if (Object.is(liveEndKey, newEndKey)) { @@ -126,7 +128,8 @@ export function reconcile( // Fallback to the slow path: we need to learn more about the content of the live and new // collections. detachedItems ??= new MultiMap(); - liveKeysInTheFuture ??= initLiveItemsInTheFuture(liveCollection, liveStartIdx, liveEndIdx); + liveKeysInTheFuture ??= + initLiveItemsInTheFuture(liveCollection, liveStartIdx, liveEndIdx, trackByFn); // Check if I'm inserting a previously detached item: if so, attach it here if (attachPreviouslyDetached(liveCollection, detachedItems, liveStartIdx, newStartKey)) { @@ -163,7 +166,8 @@ export function reconcile( while (!newIterationResult.done && liveStartIdx <= liveEndIdx) { const newValue = newIterationResult.value; const newKey = trackByFn(liveStartIdx, newValue); - const liveKey = liveCollection.key(liveStartIdx); + const liveValue = liveCollection.at(liveStartIdx); + const liveKey = trackByFn(liveStartIdx, liveValue); if (Object.is(liveKey, newKey)) { // found a match - move on liveCollection.updateValue(liveStartIdx, newValue); @@ -171,7 +175,8 @@ export function reconcile( newIterationResult = newCollectionIterator.next(); } else { detachedItems ??= new MultiMap(); - liveKeysInTheFuture ??= initLiveItemsInTheFuture(liveCollection, liveStartIdx, liveEndIdx); + liveKeysInTheFuture ??= + initLiveItemsInTheFuture(liveCollection, liveStartIdx, liveEndIdx, trackByFn); // Check if I'm inserting a previously detached item: if so, attach it here if (attachPreviouslyDetached(liveCollection, detachedItems, liveStartIdx, newKey)) { @@ -235,10 +240,11 @@ function createOrAttach( } function initLiveItemsInTheFuture( - liveCollection: LiveCollection, start: number, end: number): Set { + liveCollection: LiveCollection, start: number, end: number, + trackByFn: TrackByFunction): Set { const keys = new Set(); for (let i = start; i <= end; i++) { - keys.add(liveCollection.key(i)); + keys.add(trackByFn(i, liveCollection.at(i))); } return keys; } diff --git a/packages/core/test/render3/list_reconciliation_spec.ts b/packages/core/test/render3/list_reconciliation_spec.ts index 10e4ae07506..46fb6fc5dd1 100644 --- a/packages/core/test/render3/list_reconciliation_spec.ts +++ b/packages/core/test/render3/list_reconciliation_spec.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {TrackByFunction} from '@angular/core'; import {LiveCollection, reconcile} from '@angular/core/src/render3/list_reconciliation'; import {assertDefined} from '@angular/core/src/util/assert'; @@ -36,18 +35,15 @@ class LoggingLiveCollection extends LiveCollection { private logs: any[][] = []; constructor( - private arr: T[], private trackByFn: TrackByFunction, - private itemFactory: ItemAdapter = new NoopItemFactory()) { + private arr: T[], private itemFactory: ItemAdapter = new NoopItemFactory()) { super(); } get length(): number { return this.arr.length; } - - override key(index: number): unknown { - this.operations.key++; - return this.trackByFn(index, this.itemFactory.unwrap(this.getItem(index))); + override at(index: number): V { + return this.itemFactory.unwrap(this.getItem(index)); } override attach(index: number, item: T): void { this.logs.push(['attach', index, item]); @@ -101,7 +97,7 @@ function trackByIndex(index: number) { describe('list reconciliation', () => { describe('fast path', () => { it('should do nothing if 2 lists are the same', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, ['a', 'b', 'c'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -109,7 +105,7 @@ describe('list reconciliation', () => { }); it('should add items at the end', () => { - const pc = new LoggingLiveCollection(['a', 'b'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b']); reconcile(pc, ['a', 'b', 'c'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -117,7 +113,7 @@ describe('list reconciliation', () => { }); it('should swap items', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, ['c', 'b', 'a'], trackByIdentity); expect(pc.getCollection()).toEqual(['c', 'b', 'a']); @@ -131,7 +127,7 @@ describe('list reconciliation', () => { }); it('should should optimally swap adjacent items', () => { - const pc = new LoggingLiveCollection(['a', 'b'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b']); reconcile(pc, ['b', 'a'], trackByIdentity); expect(pc.getCollection()).toEqual(['b', 'a']); @@ -142,7 +138,7 @@ describe('list reconciliation', () => { }); it('should detect moves to the front', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']); reconcile(pc, ['a', 'd', 'b', 'c'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'd', 'b', 'c']); @@ -154,7 +150,7 @@ describe('list reconciliation', () => { it('should delete items in the middle', () => { - const pc = new LoggingLiveCollection(['a', 'x', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'x', 'b', 'c']); reconcile(pc, ['a', 'b', 'c'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -165,7 +161,7 @@ describe('list reconciliation', () => { }); it('should delete items from the beginning', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, ['c'], trackByIdentity); expect(pc.getCollection()).toEqual(['c']); @@ -178,7 +174,7 @@ describe('list reconciliation', () => { }); it('should delete items from the end', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, ['a'], trackByIdentity); expect(pc.getCollection()).toEqual(['a']); @@ -191,7 +187,7 @@ describe('list reconciliation', () => { }); it('should work with duplicated items', () => { - const pc = new LoggingLiveCollection(['a', 'a', 'a'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'a', 'a']); reconcile(pc, ['a', 'a', 'a'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'a', 'a']); @@ -201,7 +197,7 @@ describe('list reconciliation', () => { describe('slow path', () => { it('should delete multiple items from the middle', () => { - const pc = new LoggingLiveCollection(['a', 'x1', 'b', 'x2', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'x1', 'b', 'x2', 'c']); reconcile(pc, ['a', 'b', 'c'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -214,7 +210,7 @@ describe('list reconciliation', () => { }); it('should add multiple items in the middle', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, ['a', 'n1', 'b', 'n2', 'c'], trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'n1', 'b', 'n2', 'c']); @@ -227,7 +223,7 @@ describe('list reconciliation', () => { }); it('should go back to the fast path when start / end is different', () => { - const pc = new LoggingLiveCollection(['s1', 'a', 'b', 'c', 'e1'], trackByIdentity); + const pc = new LoggingLiveCollection(['s1', 'a', 'b', 'c', 'e1']); reconcile(pc, ['s2', 'a', 'b', 'c', 'e2'], trackByIdentity); expect(pc.getCollection()).toEqual(['s2', 'a', 'b', 'c', 'e2']); @@ -249,7 +245,7 @@ describe('list reconciliation', () => { }); it('should detect moves to the back', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']); reconcile(pc, ['b', 'c', 'n1', 'n2', 'n3', 'a', 'd'], trackByIdentity); expect(pc.getCollection()).toEqual(['b', 'c', 'n1', 'n2', 'n3', 'a', 'd']); expect(pc.getLogs()).toEqual([ @@ -265,7 +261,7 @@ describe('list reconciliation', () => { }); it('should create / reuse duplicated items as needed', () => { - const pc = new LoggingLiveCollection([1, 1, 2, 3], trackByIdentity); + const pc = new LoggingLiveCollection([1, 1, 2, 3]); reconcile(pc, [2, 3, 1, 1, 1, 4], trackByIdentity); expect(pc.getCollection()).toEqual([2, 3, 1, 1, 1, 4]); @@ -284,7 +280,7 @@ describe('list reconciliation', () => { describe('iterables', () => { it('should do nothing if 2 lists represented as iterables are the same', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -292,7 +288,7 @@ describe('list reconciliation', () => { }); it('should add items at the end', () => { - const pc = new LoggingLiveCollection(['a', 'b'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b']); reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -300,7 +296,7 @@ describe('list reconciliation', () => { }); it('should add multiple items in the middle', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, new Set(['a', 'n1', 'b', 'n2', 'c']), trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'n1', 'b', 'n2', 'c']); @@ -313,7 +309,7 @@ describe('list reconciliation', () => { }); it('should delete items from the end', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, new Set(['a']), trackByIdentity); expect(pc.getCollection()).toEqual(['a']); @@ -326,7 +322,7 @@ describe('list reconciliation', () => { }); it('should detect (slow) moves to the front', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']); reconcile(pc, new Set(['a', 'd', 'b', 'c']), trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'd', 'b', 'c']); @@ -339,7 +335,7 @@ describe('list reconciliation', () => { }); it('should detect (fast) moves to the back', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']); reconcile(pc, new Set(['b', 'c', 'a', 'd']), trackByIdentity); expect(pc.getCollection()).toEqual(['b', 'c', 'a', 'd']); expect(pc.getLogs()).toEqual([ @@ -349,7 +345,7 @@ describe('list reconciliation', () => { }); it('should allow switching collection types', () => { - const pc = new LoggingLiveCollection(['a', 'b', 'c'], trackByIdentity); + const pc = new LoggingLiveCollection(['a', 'b', 'c']); reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity); expect(pc.getCollection()).toEqual(['a', 'b', 'c']); @@ -394,7 +390,7 @@ describe('list reconciliation', () => { } it('should update when tracking by index - fast path from the start', () => { - const pc = new LoggingLiveCollection([], trackByIndex, new RepeaterLikeItemFactory()); + const pc = new LoggingLiveCollection([], new RepeaterLikeItemFactory()); reconcile(pc, ['a', 'b', 'c'], trackByIndex); expect(pc.getCollection()).toEqual([ @@ -413,7 +409,7 @@ describe('list reconciliation', () => { it('should update when tracking by key - fast path from the end', () => { const pc = new LoggingLiveCollection( - [], trackByKey, new RepeaterLikeItemFactory>()); + [], new RepeaterLikeItemFactory>()); reconcile(pc, [{k: 'o', v: 'o'}], trackByKey); expect(pc.getCollection()).toEqual([ @@ -430,7 +426,7 @@ describe('list reconciliation', () => { it('should update when swapping on the fast path', () => { const pc = new LoggingLiveCollection( - [], trackByKey, new RepeaterLikeItemFactory>()); + [], new RepeaterLikeItemFactory>()); reconcile(pc, [{k: 0, v: 'a'}, {k: 1, v: 'b'}, {k: 2, v: 'c'}], trackByKey as any); expect(pc.getCollection()) @@ -455,7 +451,7 @@ describe('list reconciliation', () => { it('should update when moving forward on the fast path', () => { const pc = new LoggingLiveCollection( - [], trackByKey, new RepeaterLikeItemFactory>()); + [], new RepeaterLikeItemFactory>()); reconcile( pc, [