Skip to content

Commit

Permalink
perf(core): optimize memory allocation when reconcilling lists
Browse files Browse the repository at this point in the history
This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.
  • Loading branch information
pkozlowski-opensource committed Oct 17, 2023
1 parent 7466004 commit cf9fa4b
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 38 deletions.
82 changes: 55 additions & 27 deletions packages/core/src/render3/list_reconciliation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import {TrackByFunction} from '../change_detection';
import {assertNotSame} from '../util/assert';

/**
* A type representing the live collection to be reconciled with any new (incoming) collection. This
Expand Down Expand Up @@ -73,7 +74,7 @@ export abstract class LiveCollection<T, V> {
export function reconcile<T, V>(
liveCollection: LiveCollection<T, V>, newCollection: Iterable<V>|undefined|null,
trackByFn: TrackByFunction<V>): void {
let detachedItems: MultiMap<unknown, T>|undefined = undefined;
let detachedItems: UniqueValueMultiKeyMap<unknown, T>|undefined = undefined;
let liveKeysInTheFuture: Set<unknown>|undefined = undefined;

let liveStartIdx = 0;
Expand Down Expand Up @@ -126,7 +127,7 @@ export function reconcile<T, V>(

// Fallback to the slow path: we need to learn more about the content of the live and new
// collections.
detachedItems ??= new MultiMap();
detachedItems ??= new UniqueValueMultiKeyMap();
liveKeysInTheFuture ??= initLiveItemsInTheFuture(liveCollection, liveStartIdx, liveEndIdx);

// Check if I'm inserting a previously detached item: if so, attach it here
Expand Down Expand Up @@ -171,7 +172,7 @@ export function reconcile<T, V>(
liveStartIdx++;
newIterationResult = newCollectionIterator.next();
} else {
detachedItems ??= new MultiMap();
detachedItems ??= new UniqueValueMultiKeyMap();
liveKeysInTheFuture ??= initLiveItemsInTheFuture(liveCollection, liveStartIdx, liveEndIdx);

// Check if I'm inserting a previously detached item: if so, attach it here
Expand Down Expand Up @@ -214,8 +215,9 @@ export function reconcile<T, V>(
}

function attachPreviouslyDetached<T, V>(
prevCollection: LiveCollection<T, V>, detachedItems: MultiMap<unknown, T>|undefined,
index: number, key: unknown): boolean {
prevCollection: LiveCollection<T, V>,
detachedItems: UniqueValueMultiKeyMap<unknown, T>|undefined, index: number,
key: unknown): boolean {
if (detachedItems !== undefined && detachedItems.has(key)) {
prevCollection.attach(index, detachedItems.get(key)!);
detachedItems.delete(key);
Expand All @@ -225,7 +227,8 @@ function attachPreviouslyDetached<T, V>(
}

function createOrAttach<T, V>(
liveCollection: LiveCollection<T, V>, detachedItems: MultiMap<unknown, T>|undefined,
liveCollection: LiveCollection<T, V>,
detachedItems: UniqueValueMultiKeyMap<unknown, T>|undefined,
trackByFn: TrackByFunction<unknown>, index: number, value: V) {
if (!attachPreviouslyDetached(liveCollection, detachedItems, index, trackByFn(index, value))) {
const newItem = liveCollection.create(index, value);
Expand All @@ -244,43 +247,68 @@ function initLiveItemsInTheFuture<T>(
return keys;
}

class MultiMap<K, V> {
private map = new Map<K, Array<V>>();
/**
* A specific, partial implementation of the Map interface with the following characteristics:
* - allows multiple values for a given key;
* - maintain FIFO order for multiple values corresponding to a given key;
* - assumes that all values are unique.
*
* The implementation aims at having the minimal overhead for cases where keys are _not_ duplicated
* (the most common case in the list reconciliation algorithm). To achieve this, the first value for
* a given key is stored in a regular map. Then, when more values are set for a given key, we
* maintain a kind of linked list in a separate map. To maintain this linked list we assume that all
* values (in the entire collection) are unique.
*/
export class UniqueValueMultiKeyMap<K, V> {
private kvMap = new Map<K, V>();
private _vMap: Map<V, V>|undefined;

private get vMap() {
return this._vMap ??= new Map<V, V>();
}

has(key: K): boolean {
const listOfKeys = this.map.get(key);
return listOfKeys !== undefined && listOfKeys.length > 0;
return this.kvMap.has(key);
}

delete(key: K): boolean {
const listOfKeys = this.map.get(key);
if (listOfKeys !== undefined) {
// THINK: pop from the end or shift from the front? "Correct" vs. "slow".
listOfKeys.pop();
return true;
if (!this.has(key)) return false;

const value = this.kvMap.get(key)!;
if (this.vMap.has(value)) {
this.kvMap.set(key, this.vMap.get(value)!);
this.vMap.delete(value);
} else {
this.kvMap.delete(key);
}
return false;

return true;
}

get(key: K): V|undefined {
const listOfKeys = this.map.get(key);
return listOfKeys !== undefined && listOfKeys.length > 0 ? listOfKeys[0] : undefined;
return this.kvMap.get(key);
}

set(key: K, value: V): void {
// if value is array, they we always store it as [value].
if (!this.map.has(key)) {
this.map.set(key, [value]);
return;
if (this.kvMap.has(key)) {
let prevValue = this.kvMap.get(key)!;
ngDevMode &&
assertNotSame(
prevValue, value, `Detected a duplicated value ${value} for the key ${key}`);
while (this.vMap.has(prevValue)) {
prevValue = this.vMap.get(prevValue)!;
}
this.vMap.set(prevValue, value);
} else {
this.kvMap.set(key, value);
}
// THINK: this allows duplicate values, but I guess this is fine?
// Is the existing key an array or not?
this.map.get(key)?.push(value);
}

forEach(cb: (v: V, k: K) => void) {
for (const [key, values] of this.map) {
for (const value of values) {
for (let [key, value] of this.kvMap) {
cb(value, key);
while (this.vMap.has(value)) {
value = this.vMap.get(value)!;
cb(value, key);
}
}
Expand Down
24 changes: 13 additions & 11 deletions packages/core/test/render3/list_reconciliation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,19 +263,21 @@ describe('list reconciliation', () => {
});

it('should create / reuse duplicated items as needed', () => {
const pc = new LoggingLiveCollection([1, 1, 2, 3], trackByIdentity);
reconcile(pc, [2, 3, 1, 1, 1, 4], trackByIdentity);
const trackByKey = (idx: number, item: {k: number}) => item.k;
const pc = new LoggingLiveCollection<{k: number}, {k: number}>(
[{k: 1}, {k: 1}, {k: 2}, {k: 3}], trackByKey);
reconcile(pc, [{k: 2}, {k: 3}, {k: 1}, {k: 1}, {k: 1}, {k: 4}], trackByKey);

expect(pc.getCollection()).toEqual([2, 3, 1, 1, 1, 4]);
expect(pc.getCollection()).toEqual([{k: 2}, {k: 3}, {k: 1}, {k: 1}, {k: 1}, {k: 4}]);
expect(pc.getLogs()).toEqual([
['detach', 0, 1],
['detach', 0, 1],
['attach', 2, 1],
['attach', 3, 1],
['create', 4, 1],
['attach', 4, 1],
['create', 5, 4],
['attach', 5, 4],
['detach', 0, {k: 1}],
['detach', 0, {k: 1}],
['attach', 2, {k: 1}],
['attach', 3, {k: 1}],
['create', 4, {k: 1}],
['attach', 4, {k: 1}],
['create', 5, {k: 4}],
['attach', 5, {k: 4}],
]);
});
});
Expand Down
62 changes: 62 additions & 0 deletions packages/core/test/render3/multi_map_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {UniqueValueMultiKeyMap} from '@angular/core/src/render3/list_reconciliation';

describe('MultiMap', () => {
it('should set, get and remove items with duplicated keys', () => {
const map = new UniqueValueMultiKeyMap();

map.set('k', 'v1');
map.set('k', 'v2');

expect(map.has('k')).toBeTrue();
expect(map.get('k')).toBe('v1');

map.delete('k');
expect(map.has('k')).toBeTrue();
expect(map.get('k')).toBe('v2');

map.delete('k');
expect(map.has('k')).toBeFalse();
});

it('should set, get and remove items without duplicated keys', () => {
const map = new UniqueValueMultiKeyMap();

map.set('k', 'v1');

expect(map.has('k')).toBeTrue();
expect(map.get('k')).toBe('v1');

map.delete('k');
expect(map.has('k')).toBeFalse();
});

it('should iterate with forEach', () => {
const map = new UniqueValueMultiKeyMap<string, string>();

map.set('km', 'v1');
map.set('km', 'v2');
map.set('ks', 'v');

const items: string[][] = [];

map.forEach((v, k) => items.push([v, k]));
expect(items).toEqual([['v1', 'km'], ['v2', 'km'], ['v', 'ks']]);
});

it('should throw upon detecting duplicate values', () => {
const map = new UniqueValueMultiKeyMap();

map.set('k', 'v');
expect(() => {
map.set('k', 'v');
}).toThrowError(/Detected a duplicated value v for the key k/);
});
});

0 comments on commit cf9fa4b

Please sign in to comment.