Skip to content

Commit 731ae45

Browse files
secitrmeta-codesync[bot]
authored andcommitted
Reduce allocations in VirtualizedList render and scroll path (#58593)
Summary: `VirtualizedList`/`FlatList`/`SectionList` re-run a small amount of bookkeeping on every render and on every scroll event (60–120 Hz on ProMotion displays). This PR removes three allocations from those hot paths without changing any observable behavior: 1. **`VirtualizedList.render` no longer builds a `Set` for `stickyHeaderIndices` on every render** when the prop is not provided (the common case). The Set is now only created when the prop is present; the two `.has()` lookups use optional access. 2. **`ChildListCollection.forEach` returns early when there are no nested child lists** (the common case) instead of allocating a `Map.values()` iterator. This is called from `_onScroll` and the four other scroll callbacks on every scroll event. 3. **`_orientation()` caches its result** and only rebuilds the object when the `horizontal` prop changes. `I18nManager.isRTL` is a module-load constant (only changes on app reload), so the cache is invalidated solely by the `horizontal` prop. The object is replaced, never mutated, which keeps `ListMetricsAggregator`'s field-based invalidation correct. ## Changelog: [GENERAL][CHANGED] - Reduce allocations in the `VirtualizedList` render and scroll path (avoid per-render `Set` allocation for `stickyHeaderIndices`, per-scroll-event `Map` iterator for the empty nested-list collection, and per-call `orientation` object allocation) Pull Request resolved: #58593 Test Plan: - `yarn test packages/virtualized-lists` → 9 suites, 186 passed, 69 snapshots: - `ChildListCollection-test.js` (new): forEach over populated/empty collection, removal, `forEachInCell`/`anyInCell` - `VirtualizedList-test.js`: `stickyHeaderIndices` not forwarded when the prop is absent (with `ListHeaderComponent`), forwarded when provided; orientation cache identity + invalidation on `horizontal` change - `yarn flow-check` → 0 errors - `yarn lint` → 0 errors, 0 warnings - `yarn format-check` (changed files) Micro-benchmark (Node v24, V8, 2M iterations, before vs after, same machine; the real-world benefit is dominated by reduced GC pressure, which is largest on low-end Android): Reviewed By: Abbondanzo Differential Revision: D121177373 Pulled By: javache fbshipit-source-id: 1befd7e6d90fbd1fcea9c4cc8cf6d575c6c26111
1 parent 7ce1b2c commit 731ae45

4 files changed

Lines changed: 130 additions & 19 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/VirtualizedList.js‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,7 @@ class VirtualizedList extends StateSafePureComponent<
255255
return;
256256
}
257257

258-
const {horizontal, rtl} = this._orientation();
259-
if (horizontal && rtl && !this._listMetrics.hasContentLength()) {
258+
if (this._isHorizontalRTL() && !this._listMetrics.hasContentLength()) {
260259
console.warn(
261260
'scrollToOffset may not be called in RTL before content is laid out',
262261
);
@@ -277,9 +276,7 @@ class VirtualizedList extends StateSafePureComponent<
277276
const cartOffset = this._listMetrics.cartesianOffset(
278277
offset + this._scrollMetrics.visibleLength,
279278
);
280-
/* $FlowFixMe[constant-condition] Error discovered during Constant
281-
* Condition roll out. See https://fburl.com/workplace/1v97vimq. */
282-
return horizontal ? {x: cartOffset} : {y: cartOffset};
279+
return {x: cartOffset};
283280
} else {
284281
return horizontal ? {x: offset} : {y: offset};
285282
}
@@ -437,13 +434,6 @@ class VirtualizedList extends StateSafePureComponent<
437434
'VirtualizedList: The windowSize prop must be present and set to a value greater than 0.',
438435
);
439436

440-
invariant(
441-
/* $FlowFixMe[constant-condition] Error discovered during Constant
442-
* Condition roll out. See https://fburl.com/workplace/1v97vimq. */
443-
getItemCount,
444-
'VirtualizedList: The "getItemCount" prop must be provided',
445-
);
446-
447437
const itemCount = getItemCount(data);
448438

449439
if (
@@ -786,7 +776,7 @@ class VirtualizedList extends StateSafePureComponent<
786776
_pushCells(
787777
cells: Array<Object>,
788778
stickyHeaderIndices: Array<number>,
789-
stickyIndicesFromProps: Set<number>,
779+
stickyIndicesFromProps: ?Set<number>,
790780
first: number,
791781
last: number,
792782
inversionStyle: StyleProp<ViewStyle>,
@@ -814,7 +804,7 @@ class VirtualizedList extends StateSafePureComponent<
814804
const key = VirtualizedList._keyExtractor(item, ii, this.props);
815805

816806
this._indicesToKeys.set(ii, key);
817-
if (stickyIndicesFromProps.has(ii + stickyOffset)) {
807+
if (stickyIndicesFromProps?.has(ii + stickyOffset)) {
818808
stickyHeaderIndices.push(cells.length);
819809
}
820810

@@ -945,12 +935,16 @@ class VirtualizedList extends StateSafePureComponent<
945935
: styles.verticallyInverted
946936
: null;
947937
const cells: Array<any | React.Node> = [];
948-
const stickyIndicesFromProps = new Set(this.props.stickyHeaderIndices);
938+
// Avoid allocating a Set on every render when no sticky headers are
939+
// configured (the common case).
940+
const stickyHeaderIndicesProp = this.props.stickyHeaderIndices;
941+
const stickyIndicesFromProps =
942+
stickyHeaderIndicesProp != null ? new Set(stickyHeaderIndicesProp) : null;
949943
const stickyHeaderIndices = [];
950944

951945
// 1. Add cell for ListHeaderComponent
952946
if (ListHeaderComponent) {
953-
if (stickyIndicesFromProps.has(0)) {
947+
if (stickyIndicesFromProps?.has(0)) {
954948
stickyHeaderIndices.push(0);
955949
}
956950
const element = isValidElement(ListHeaderComponent) ? (
@@ -1549,7 +1543,11 @@ class VirtualizedList extends StateSafePureComponent<
15491543
}
15501544

15511545
_selectOffset({x, y}: Readonly<{x: number, y: number, ...}>): number {
1552-
return this._orientation().horizontal ? x : y;
1546+
return horizontalOrDefault(this.props.horizontal) ? x : y;
1547+
}
1548+
1549+
_isHorizontalRTL(): boolean {
1550+
return horizontalOrDefault(this.props.horizontal) && I18nManager.isRTL;
15531551
}
15541552

15551553
_orientation(): ListOrientation {
@@ -1802,8 +1800,7 @@ class VirtualizedList extends StateSafePureComponent<
18021800

18031801
_offsetFromScrollEvent(e: ScrollEvent): number {
18041802
const {contentOffset, contentSize, layoutMeasurement} = e.nativeEvent;
1805-
const {horizontal, rtl} = this._orientation();
1806-
if (horizontal && rtl) {
1803+
if (this._isHorizontalRTL()) {
18071804
return (
18081805
this._selectLength(contentSize) -
18091806
(this._selectOffset(contentOffset) +
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
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 => {
24+
visited.push(list);
25+
});
26+
expect(visited.sort()).toEqual(['a', 'b', 'c']);
27+
expect(collection.size()).toBe(3);
28+
});
29+
30+
it('does not call the callback when the collection is empty', function () {
31+
const collection = new ChildListCollection<string>();
32+
const callback = jest.fn();
33+
collection.forEach(callback);
34+
expect(callback).not.toHaveBeenCalled();
35+
expect(collection.size()).toBe(0);
36+
});
37+
38+
it('stops iterating entries after they are removed', function () {
39+
const collection = new ChildListCollection<string>();
40+
collection.add('a', 'cell1');
41+
collection.remove('a');
42+
43+
const visited = [];
44+
collection.forEach(list => {
45+
visited.push(list);
46+
});
47+
expect(visited).toEqual([]);
48+
expect(collection.size()).toBe(0);
49+
});
50+
51+
it('supports forEachInCell and anyInCell', function () {
52+
const collection = new ChildListCollection<string>();
53+
collection.add('a', 'cell1');
54+
collection.add('b', 'cell2');
55+
56+
const visited = [];
57+
collection.forEachInCell('cell1', list => {
58+
visited.push(list);
59+
});
60+
expect(visited).toEqual(['a']);
61+
62+
expect(collection.anyInCell('cell2', list => list === 'b')).toBe(true);
63+
expect(collection.anyInCell('cell1', list => list === 'b')).toBe(false);
64+
expect(collection.anyInCell('missing', () => true)).toBe(false);
65+
});
66+
});

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,49 @@ 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+
10551098
it('does not add a sticky header to the render mask when no sticky headers are configured', () => {
10561099
const expectedRegions = [
10571100
{first: 0, last: 9, isSpacer: true},

0 commit comments

Comments
 (0)