Skip to content

Commit 5cfd2e2

Browse files
author
sedat.ciftci
committed
Reduce allocations in VirtualizedList render and scroll path
1 parent c687a35 commit 5cfd2e2

6 files changed

Lines changed: 214 additions & 13 deletions

File tree

packages/virtualized-lists/Lists/ChildListCollection.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ export default class ChildListCollection<TList> {
4242
}
4343

4444
forEach(fn: TList => void): void {
45+
// Fast-path for the common case of a list without nested child lists,
46+
// which avoids allocating a Map iterator on every scroll event.
47+
if (this._cellKeyToChildren.size === 0) {
48+
return;
49+
}
4550
for (const listSet of this._cellKeyToChildren.values()) {
4651
for (const list of listSet) {
4752
fn(list);

packages/virtualized-lists/Lists/VirtualizeUtils.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,13 @@ export function computeWindowedRenderLimits(
244244
}
245245

246246
export function keyExtractor(item: any, index: number): string {
247-
if (typeof item === 'object' && item?.key != null) {
248-
return item.key;
249-
}
250-
if (typeof item === 'object' && item?.id != null) {
251-
return item.id;
247+
if (item != null && typeof item === 'object') {
248+
if (item.key != null) {
249+
return item.key;
250+
}
251+
if (item.id != null) {
252+
return item.id;
253+
}
252254
}
253255
return String(index);
254256
}

packages/virtualized-lists/Lists/VirtualizedList.js

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ class VirtualizedList extends StateSafePureComponent<
785785
_pushCells(
786786
cells: Array<Object>,
787787
stickyHeaderIndices: Array<number>,
788-
stickyIndicesFromProps: Set<number>,
788+
stickyIndicesFromProps: null | Set<number>,
789789
first: number,
790790
last: number,
791791
inversionStyle: StyleProp<ViewStyle>,
@@ -813,7 +813,10 @@ class VirtualizedList extends StateSafePureComponent<
813813
const key = VirtualizedList._keyExtractor(item, ii, this.props);
814814

815815
this._indicesToKeys.set(ii, key);
816-
if (stickyIndicesFromProps.has(ii + stickyOffset)) {
816+
if (
817+
stickyIndicesFromProps != null &&
818+
stickyIndicesFromProps.has(ii + stickyOffset)
819+
) {
817820
stickyHeaderIndices.push(cells.length);
818821
}
819822

@@ -944,12 +947,16 @@ class VirtualizedList extends StateSafePureComponent<
944947
: styles.verticallyInverted
945948
: null;
946949
const cells: Array<any | React.Node> = [];
947-
const stickyIndicesFromProps = new Set(this.props.stickyHeaderIndices);
950+
// Avoid allocating a Set on every render when no sticky headers are
951+
// configured (the common case).
952+
const stickyHeaderIndicesProp = this.props.stickyHeaderIndices;
953+
const stickyIndicesFromProps =
954+
stickyHeaderIndicesProp != null ? new Set(stickyHeaderIndicesProp) : null;
948955
const stickyHeaderIndices = [];
949956

950957
// 1. Add cell for ListHeaderComponent
951958
if (ListHeaderComponent) {
952-
if (stickyIndicesFromProps.has(0)) {
959+
if (stickyIndicesFromProps != null && stickyIndicesFromProps.has(0)) {
953960
stickyHeaderIndices.push(0);
954961
}
955962
const element = isValidElement(ListHeaderComponent) ? (
@@ -1231,6 +1238,8 @@ class VirtualizedList extends StateSafePureComponent<
12311238
}
12321239
}
12331240

1241+
_cachedOrientation: ?ListOrientation = null;
1242+
_cachedOrientationHorizontal: ?boolean = null;
12341243
_cellRefs: {[string]: null | CellRenderer<any>} = {};
12351244
_fillRateHelper: FillRateHelper;
12361245
_listMetrics: ListMetricsAggregator = new ListMetricsAggregator();
@@ -1552,10 +1561,23 @@ class VirtualizedList extends StateSafePureComponent<
15521561
}
15531562

15541563
_orientation(): ListOrientation {
1555-
return {
1556-
horizontal: horizontalOrDefault(this.props.horizontal),
1557-
rtl: I18nManager.isRTL,
1558-
};
1564+
// The orientation is stable for the lifetime of the list unless the
1565+
// `horizontal` prop changes (I18nManager.isRTL only changes on app
1566+
// reload). Cache the object to avoid allocating it on the scroll path.
1567+
const horizontal = horizontalOrDefault(this.props.horizontal);
1568+
let cachedOrientation = this._cachedOrientation;
1569+
if (
1570+
cachedOrientation == null ||
1571+
this._cachedOrientationHorizontal !== horizontal
1572+
) {
1573+
cachedOrientation = {
1574+
horizontal,
1575+
rtl: I18nManager.isRTL,
1576+
};
1577+
this._cachedOrientation = cachedOrientation;
1578+
this._cachedOrientationHorizontal = horizontal;
1579+
}
1580+
return cachedOrientation;
15591581
}
15601582

15611583
_maybeCallOnEdgeReached() {
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
'use strict';
12+
13+
import ChildListCollection from '../ChildListCollection';
14+
15+
describe('ChildListCollection', function () {
16+
it('iterates over all child lists with forEach', function () {
17+
const collection = new ChildListCollection<string>();
18+
collection.add('a', 'cell1');
19+
collection.add('b', 'cell1');
20+
collection.add('c', 'cell2');
21+
22+
const visited = [];
23+
collection.forEach(list => visited.push(list));
24+
expect(visited.sort()).toEqual(['a', 'b', 'c']);
25+
expect(collection.size()).toBe(3);
26+
});
27+
28+
it('does not call the callback when the collection is empty', function () {
29+
const collection = new ChildListCollection<string>();
30+
const callback = jest.fn();
31+
collection.forEach(callback);
32+
expect(callback).not.toHaveBeenCalled();
33+
expect(collection.size()).toBe(0);
34+
});
35+
36+
it('stops iterating entries after they are removed', function () {
37+
const collection = new ChildListCollection<string>();
38+
collection.add('a', 'cell1');
39+
collection.remove('a');
40+
41+
const visited = [];
42+
collection.forEach(list => visited.push(list));
43+
expect(visited).toEqual([]);
44+
expect(collection.size()).toBe(0);
45+
});
46+
47+
it('supports forEachInCell and anyInCell', function () {
48+
const collection = new ChildListCollection<string>();
49+
collection.add('a', 'cell1');
50+
collection.add('b', 'cell2');
51+
52+
const visited = [];
53+
collection.forEachInCell('cell1', list => visited.push(list));
54+
expect(visited).toEqual(['a']);
55+
56+
expect(collection.anyInCell('cell2', list => list === 'b')).toBe(true);
57+
expect(collection.anyInCell('cell1', list => list === 'b')).toBe(false);
58+
expect(collection.anyInCell('missing', () => true)).toBe(false);
59+
});
60+
});

packages/virtualized-lists/Lists/__tests__/VirtualizeUtils-test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import ListMetricsAggregator from '../ListMetricsAggregator';
1616
import {
1717
computeWindowedRenderLimits,
1818
elementsThatOverlapOffsets,
19+
keyExtractor,
1920
newRangeCount,
2021
} from '../VirtualizeUtils';
2122
import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags';
@@ -292,3 +293,35 @@ describe('computeWindowedRenderLimits', function () {
292293
expect(result).toEqual({first: 0, last: 4});
293294
});
294295
});
296+
297+
describe('keyExtractor', function () {
298+
it('prefers item.key', function () {
299+
expect(keyExtractor({key: 'k', id: 1}, 0)).toBe('k');
300+
});
301+
302+
it('falls back to item.id when key is missing', function () {
303+
expect(keyExtractor({id: 42}, 0)).toBe(42);
304+
});
305+
306+
it('treats explicit null key as missing', function () {
307+
expect(keyExtractor({key: null, id: 9}, 0)).toBe(9);
308+
});
309+
310+
it('returns explicitly set falsy key and id values', function () {
311+
expect(keyExtractor({key: 0}, 0)).toBe(0);
312+
expect(keyExtractor({key: false}, 0)).toBe(false);
313+
expect(keyExtractor({key: null, id: 0}, 0)).toBe(0);
314+
});
315+
316+
it('falls back to the index for items without key or id', function () {
317+
expect(keyExtractor({}, 7)).toBe('7');
318+
});
319+
320+
it('falls back to the index for null, undefined, primitives and arrays', function () {
321+
expect(keyExtractor(null, 1)).toBe('1');
322+
expect(keyExtractor(undefined, 2)).toBe('2');
323+
expect(keyExtractor('str', 3)).toBe('3');
324+
expect(keyExtractor(42, 4)).toBe('4');
325+
expect(keyExtractor([], 5)).toBe('5');
326+
});
327+
});

packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,85 @@ describe('VirtualizedList', () => {
10521052
expect(component).toMatchSnapshot();
10531053
});
10541054

1055+
it('does not forward stickyHeaderIndices when the prop is absent', async () => {
1056+
let scrollProps;
1057+
await act(() => {
1058+
create(
1059+
<VirtualizedList
1060+
ListHeaderComponent={() => createElement('Header')}
1061+
data={[{key: 'i1'}, {key: 'i2'}]}
1062+
renderItem={({item}) => <item value={item.key} />}
1063+
getItem={(data, index) => data[index]}
1064+
getItemCount={data => data.length}
1065+
renderScrollComponent={props => {
1066+
scrollProps = props;
1067+
return createElement('MockScrollView', props);
1068+
}}
1069+
/>,
1070+
);
1071+
});
1072+
expect(scrollProps).not.toBe(undefined);
1073+
expect(scrollProps.stickyHeaderIndices).toEqual([]);
1074+
});
1075+
1076+
it('forwards stickyHeaderIndices including the header index when provided', async () => {
1077+
let scrollProps;
1078+
await act(() => {
1079+
create(
1080+
<VirtualizedList
1081+
ListHeaderComponent={() => createElement('Header')}
1082+
data={[{key: 'i1'}, {key: 'i2'}]}
1083+
renderItem={({item}) => <item value={item.key} />}
1084+
getItem={(data, index) => data[index]}
1085+
getItemCount={data => data.length}
1086+
stickyHeaderIndices={[0]}
1087+
renderScrollComponent={props => {
1088+
scrollProps = props;
1089+
return createElement('MockScrollView', props);
1090+
}}
1091+
/>,
1092+
);
1093+
});
1094+
expect(scrollProps).not.toBe(undefined);
1095+
expect(scrollProps.stickyHeaderIndices).toEqual([0]);
1096+
});
1097+
1098+
it('caches orientation and invalidates the cache when horizontal changes', async () => {
1099+
let component;
1100+
await act(() => {
1101+
component = create(
1102+
<VirtualizedList
1103+
data={[{key: 'i1'}]}
1104+
renderItem={({item}) => <item value={item.key} />}
1105+
getItem={(data, index) => data[index]}
1106+
getItemCount={data => data.length}
1107+
/>,
1108+
);
1109+
});
1110+
1111+
const instance = component.getInstance();
1112+
const firstOrientation = instance._orientation();
1113+
expect(instance._orientation()).toBe(firstOrientation);
1114+
expect(firstOrientation.horizontal).toBe(false);
1115+
1116+
await act(() => {
1117+
component.update(
1118+
<VirtualizedList
1119+
horizontal={true}
1120+
data={[{key: 'i1'}]}
1121+
renderItem={({item}) => <item value={item.key} />}
1122+
getItem={(data, index) => data[index]}
1123+
getItemCount={data => data.length}
1124+
/>,
1125+
);
1126+
});
1127+
1128+
const secondOrientation = instance._orientation();
1129+
expect(secondOrientation).not.toBe(firstOrientation);
1130+
expect(secondOrientation.horizontal).toBe(true);
1131+
expect(instance._orientation()).toBe(secondOrientation);
1132+
});
1133+
10551134
it('does not add a sticky header to the render mask when no sticky headers are configured', () => {
10561135
const expectedRegions = [
10571136
{first: 0, last: 9, isSpacer: true},

0 commit comments

Comments
 (0)