Skip to content

Commit 2ce3e7a

Browse files
eps1loneps1lon
committed
Stop creating Owner Stacks if many have been created recently
Adding Owner Stacks in development adds some non-negligible performance overhead. This is shared roughly equally between the work required for `captureOwnerStack` to work and tasks being visible in runtimes that support `console.createTask`. We now stop this work after a some amount of elements were created. Though we do reset that limit occasionally so that one-off updates on not too large trees do get complete Owner Stacks. Chances are that you probably don't need Owner Stacks on large, frequent updates. Stopping after 10.000 elements caps a large update at 500ms where uncapped would've taken 3.000ms (see attached fixture). I added separate, dynamic feature flags to control when we cut off and how frequent we reset so that Meta can experiment with different values.
1 parent 1a25861 commit 2ce3e7a

11 files changed

+174
-9
lines changed

‎packages/react-reconciler/src/__tests__/ReactOwnerStacks-test.js

+79
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,83 @@ describe('ReactOwnerStacks', () => {
8585
expect(React.captureOwnerStack()).toBe(null);
8686
}
8787
});
88+
89+
// @gate __DEV__
90+
it('cuts off at the owner stack limit', async () => {
91+
function App({siblingsBeforeStackOne}) {
92+
const children = [];
93+
for (
94+
let i = 0;
95+
i <
96+
siblingsBeforeStackOne -
97+
// Number of JSX callsites before this render
98+
1 -
99+
// Stop so that OwnerStackOne will be right before cutoff
100+
1;
101+
i++
102+
) {
103+
children.push(<Component key={i} />);
104+
}
105+
children.push(<OwnerStackOne key="stackOne" />);
106+
children.push(<OwnerStackTwo key="stackTwo" />);
107+
108+
return children;
109+
}
110+
111+
function Component() {
112+
return null;
113+
}
114+
115+
let stackOne;
116+
function OwnerStackOne() {
117+
stackOne = React.captureOwnerStack();
118+
}
119+
120+
let stackTwo;
121+
function OwnerStackTwo() {
122+
stackTwo = React.captureOwnerStack();
123+
}
124+
125+
await act(() => {
126+
ReactNoop.render(
127+
<App
128+
key="one"
129+
// Should be the value with of `ownerStackLimit` with `__VARIANT__` so that we see the cutoff
130+
siblingsBeforeStackOne={500}
131+
/>,
132+
);
133+
});
134+
135+
expect({
136+
stackOne: normalizeCodeLocInfo(stackOne),
137+
stackTwo: normalizeCodeLocInfo(stackTwo),
138+
}).toEqual({
139+
stackOne: '\n in App (at **)',
140+
stackTwo: __VARIANT__
141+
? // captured right after cutoff
142+
''
143+
: '\n in App (at **)',
144+
});
145+
146+
// Our internal act flushes pending timers, so this will render with owner
147+
// stacks intact until we hit the limit again.
148+
await act(() => {
149+
ReactNoop.render(
150+
<App
151+
// TODO: Owner Stacks should update on re-render.
152+
key="two"
153+
siblingsBeforeStackOne={499}
154+
/>,
155+
);
156+
});
157+
158+
expect({
159+
stackOne: normalizeCodeLocInfo(stackOne),
160+
stackTwo: normalizeCodeLocInfo(stackTwo),
161+
}).toEqual({
162+
// rendered everything before cutoff
163+
stackOne: '\n in App (at **)',
164+
stackTwo: '\n in App (at **)',
165+
});
166+
});
88167
});

‎packages/react/src/jsx/ReactJSXElement.js

+53-9
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,19 @@ import {
1616
} from 'shared/ReactSymbols';
1717
import {checkKeyStringCoercion} from 'shared/CheckStringCoercion';
1818
import isArray from 'shared/isArray';
19-
import {disableDefaultPropsExceptForClasses} from 'shared/ReactFeatureFlags';
19+
import {
20+
disableDefaultPropsExceptForClasses,
21+
debugInfoLimitsResetIntervalMs,
22+
debugTaskLimit,
23+
ownerStackLimit,
24+
} from 'shared/ReactFeatureFlags';
25+
26+
let recentlyCreatedOwnerStacks = 0;
27+
let recentlyCreatedDebugTasks = 0;
28+
setInterval(() => {
29+
recentlyCreatedOwnerStacks = 0;
30+
recentlyCreatedDebugTasks = 0;
31+
}, debugInfoLimitsResetIntervalMs);
2032

2133
const createTask =
2234
// eslint-disable-next-line react-internal/no-production-logging
@@ -380,8 +392,16 @@ export function jsxProdSignatureRunningInDevWithDynamicChildren(
380392
isStaticChildren,
381393
source,
382394
self,
383-
__DEV__ && Error('react-stack-top-frame'),
384-
__DEV__ && createTask(getTaskName(type)),
395+
// TODO: Reuse a placeholder stack to indicate the deopt.
396+
__DEV__ &&
397+
(recentlyCreatedOwnerStacks++ < ownerStackLimit
398+
? Error('react-stack-top-frame')
399+
: null),
400+
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
401+
__DEV__ &&
402+
(recentlyCreatedDebugTasks++ < debugTaskLimit
403+
? createTask(getTaskName(type))
404+
: null),
385405
);
386406
}
387407
}
@@ -402,8 +422,16 @@ export function jsxProdSignatureRunningInDevWithStaticChildren(
402422
isStaticChildren,
403423
source,
404424
self,
405-
__DEV__ && Error('react-stack-top-frame'),
406-
__DEV__ && createTask(getTaskName(type)),
425+
// TODO: Reuse a placeholder stack to indicate the deopt.
426+
__DEV__ &&
427+
(recentlyCreatedOwnerStacks++ < ownerStackLimit
428+
? Error('react-stack-top-frame')
429+
: null),
430+
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
431+
__DEV__ &&
432+
(recentlyCreatedDebugTasks++ < debugTaskLimit
433+
? createTask(getTaskName(type))
434+
: null),
407435
);
408436
}
409437
}
@@ -424,8 +452,16 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) {
424452
isStaticChildren,
425453
source,
426454
self,
427-
__DEV__ && Error('react-stack-top-frame'),
428-
__DEV__ && createTask(getTaskName(type)),
455+
// TODO: Reuse a placeholder stack to indicate the deopt.
456+
__DEV__ &&
457+
(recentlyCreatedOwnerStacks++ < ownerStackLimit
458+
? Error('react-stack-top-frame')
459+
: null),
460+
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
461+
__DEV__ &&
462+
(recentlyCreatedDebugTasks++ < debugTaskLimit
463+
? createTask(getTaskName(type))
464+
: null),
429465
);
430466
}
431467

@@ -700,8 +736,16 @@ export function createElement(type, config, children) {
700736
undefined,
701737
getOwner(),
702738
props,
703-
__DEV__ && Error('react-stack-top-frame'),
704-
__DEV__ && createTask(getTaskName(type)),
739+
// TODO: Reuse a placeholder stack to indicate the deopt.
740+
__DEV__ &&
741+
(recentlyCreatedOwnerStacks++ < ownerStackLimit
742+
? Error('react-stack-top-frame')
743+
: null),
744+
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
745+
__DEV__ &&
746+
(recentlyCreatedDebugTasks++ < debugTaskLimit
747+
? createTask(getTaskName(type))
748+
: null),
705749
);
706750
}
707751

‎packages/shared/ReactFeatureFlags.js

+4
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,7 @@ export const enableUpdaterTracking = __PROFILE__;
267267

268268
// Internal only.
269269
export const enableDO_NOT_USE_disableStrictPassiveEffect = false;
270+
271+
export const ownerStackLimit = 1e4;
272+
export const debugTaskLimit = 1e4;
273+
export const debugInfoLimitsResetIntervalMs = 1000;

‎packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js

+9
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,12 @@ export const enableSiblingPrerendering = __VARIANT__;
2828
export const enableUseEffectCRUDOverload = __VARIANT__;
2929
export const enableFastAddPropertiesInDiffing = __VARIANT__;
3030
export const enableLazyPublicInstanceInFabric = __VARIANT__;
31+
export const ownerStackLimit: number = __VARIANT__
32+
? // Some value that doesn't impact existing tests
33+
500
34+
: 1e4;
35+
export const debugTaskLimit: number = __VARIANT__
36+
? // Some value that doesn't impact existing tests
37+
500
38+
: 1e4;
39+
export const debugInfoLimitsResetIntervalMs: number = __VARIANT__ ? 1000 : 2000;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

+3
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ export const {
3030
enableSiblingPrerendering,
3131
enableFastAddPropertiesInDiffing,
3232
enableLazyPublicInstanceInFabric,
33+
ownerStackLimit,
34+
debugTaskLimit,
35+
debugInfoLimitsResetIntervalMs,
3336
} = dynamicFlags;
3437

3538
// The rest of the flags are static for better dead code elimination.

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

+3
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ export const enableSwipeTransition = false;
7575
export const enableFastAddPropertiesInDiffing = false;
7676
export const enableLazyPublicInstanceInFabric = false;
7777
export const enableScrollEndPolyfill = true;
78+
export const ownerStackLimit = 1e4;
79+
export const debugTaskLimit = 1e4;
80+
export const debugInfoLimitsResetIntervalMs = 1000;
7881

7982
// Profiling Only
8083
export const enableProfilerTimer = __PROFILE__;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

+3
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ export const enableSwipeTransition = false;
7575
export const enableFastAddPropertiesInDiffing = true;
7676
export const enableLazyPublicInstanceInFabric = false;
7777
export const enableScrollEndPolyfill = true;
78+
export const ownerStackLimit = 1e4;
79+
export const debugTaskLimit = 1e4;
80+
export const debugInfoLimitsResetIntervalMs = 1000;
7881

7982
// TODO: This must be in sync with the main ReactFeatureFlags file because
8083
// the Test Renderer's value must be the same as the one used by the

‎packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js

+3
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ export const enableSwipeTransition = false;
7171
export const enableFastAddPropertiesInDiffing = false;
7272
export const enableLazyPublicInstanceInFabric = false;
7373
export const enableScrollEndPolyfill = true;
74+
export const ownerStackLimit = 1e4;
75+
export const debugTaskLimit = 1e4;
76+
export const debugInfoLimitsResetIntervalMs = 1000;
7477

7578
// Flow magic to verify the exports of this file match the original version.
7679
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

+4
Original file line numberDiff line numberDiff line change
@@ -87,5 +87,9 @@ export const enableFastAddPropertiesInDiffing = false;
8787
export const enableLazyPublicInstanceInFabric = false;
8888
export const enableScrollEndPolyfill = true;
8989

90+
export const ownerStackLimit = 1e4;
91+
export const debugTaskLimit = 1e4;
92+
export const debugInfoLimitsResetIntervalMs = 1000;
93+
9094
// Flow magic to verify the exports of this file match the original version.
9195
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

‎packages/shared/forks/ReactFeatureFlags.www-dynamic.js

+10
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ export const enableLazyPublicInstanceInFabric = false;
4141
export const enableViewTransition = __VARIANT__;
4242
export const enableScrollEndPolyfill = __VARIANT__;
4343

44+
export const ownerStackLimit: number = __VARIANT__
45+
? // Some value that doesn't impact existing tests
46+
500
47+
: 1e4;
48+
export const debugTaskLimit: number = __VARIANT__
49+
? // Some value that doesn't impact existing tests
50+
500
51+
: 1e4;
52+
export const debugInfoLimitsResetIntervalMs: number = __VARIANT__ ? 1000 : 2000;
53+
4454
// TODO: These flags are hard-coded to the default values used in open source.
4555
// Update the tests so that they pass in either mode, then set these
4656
// to __VARIANT__.

‎packages/shared/forks/ReactFeatureFlags.www.js

+3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export const {
3838
enableFastAddPropertiesInDiffing,
3939
enableViewTransition,
4040
enableScrollEndPolyfill,
41+
ownerStackLimit,
42+
debugTaskLimit,
43+
debugInfoLimitsResetIntervalMs,
4144
} = dynamicFeatureFlags;
4245

4346
// On WWW, __EXPERIMENTAL__ is used for a new modern build.

0 commit comments

Comments
 (0)