Skip to content

Commit

Permalink
Stop creating Owner Stacks if many have been created recently
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
eps1lon committed Mar 5, 2025
1 parent 73e33e8 commit edcfcb8
Show file tree
Hide file tree
Showing 11 changed files with 174 additions and 9 deletions.
79 changes: 79 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactOwnerStacks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,83 @@ describe('ReactOwnerStacks', () => {
expect(React.captureOwnerStack()).toBe(null);
}
});

// @gate __DEV__
it('cuts off at the owner stack limit', async () => {
function App({siblingsBeforeStackOne}) {
const children = [];
for (
let i = 0;
i <
siblingsBeforeStackOne -
// Number of JSX callsites before this render
1 -
// Stop so that OwnerStackOne will be right before cutoff
1;
i++
) {
children.push(<Component key={i} />);
}
children.push(<OwnerStackOne key="stackOne" />);
children.push(<OwnerStackTwo key="stackTwo" />);

return children;
}

function Component() {
return null;
}

let stackOne;
function OwnerStackOne() {
stackOne = React.captureOwnerStack();
}

let stackTwo;
function OwnerStackTwo() {
stackTwo = React.captureOwnerStack();
}

await act(() => {
ReactNoop.render(
<App
key="one"
// Should be the value with of `ownerStackLimit` with `__VARIANT__` so that we see the cutoff
siblingsBeforeStackOne={500}
/>,
);
});

expect({
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
stackOne: '\n in App (at **)',
stackTwo: __VARIANT__
? // captured right after cutoff
''
: '\n in App (at **)',
});

// Our internal act flushes pending timers, so this will render with owner
// stacks intact until we hit the limit again.
await act(() => {
ReactNoop.render(
<App
// TODO: Owner Stacks should update on re-render.
key="two"
siblingsBeforeStackOne={499}
/>,
);
});

expect({
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// rendered everything before cutoff
stackOne: '\n in App (at **)',
stackTwo: '\n in App (at **)',
});
});
});
62 changes: 53 additions & 9 deletions packages/react/src/jsx/ReactJSXElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,19 @@ import {
} from 'shared/ReactSymbols';
import {checkKeyStringCoercion} from 'shared/CheckStringCoercion';
import isArray from 'shared/isArray';
import {disableDefaultPropsExceptForClasses} from 'shared/ReactFeatureFlags';
import {
disableDefaultPropsExceptForClasses,
debugInfoLimitsResetIntervalMs,
debugTaskLimit,
ownerStackLimit,
} from 'shared/ReactFeatureFlags';

let recentlyCreatedOwnerStacks = 0;
let recentlyCreatedDebugTasks = 0;
setInterval(() => {
recentlyCreatedOwnerStacks = 0;
recentlyCreatedDebugTasks = 0;
}, debugInfoLimitsResetIntervalMs);

const createTask =
// eslint-disable-next-line react-internal/no-production-logging
Expand Down Expand Up @@ -380,8 +392,16 @@ export function jsxProdSignatureRunningInDevWithDynamicChildren(
isStaticChildren,
source,
self,
__DEV__ && Error('react-stack-top-frame'),
__DEV__ && createTask(getTaskName(type)),
// TODO: Reuse a placeholder stack to indicate the deopt.
__DEV__ &&
(recentlyCreatedOwnerStacks++ < ownerStackLimit
? Error('react-stack-top-frame')
: null),
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
__DEV__ &&
(recentlyCreatedDebugTasks++ < debugTaskLimit
? createTask(getTaskName(type))
: null),
);
}
}
Expand All @@ -402,8 +422,16 @@ export function jsxProdSignatureRunningInDevWithStaticChildren(
isStaticChildren,
source,
self,
__DEV__ && Error('react-stack-top-frame'),
__DEV__ && createTask(getTaskName(type)),
// TODO: Reuse a placeholder stack to indicate the deopt.
__DEV__ &&
(recentlyCreatedOwnerStacks++ < ownerStackLimit
? Error('react-stack-top-frame')
: null),
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
__DEV__ &&
(recentlyCreatedDebugTasks++ < debugTaskLimit
? createTask(getTaskName(type))
: null),
);
}
}
Expand All @@ -424,8 +452,16 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) {
isStaticChildren,
source,
self,
__DEV__ && Error('react-stack-top-frame'),
__DEV__ && createTask(getTaskName(type)),
// TODO: Reuse a placeholder stack to indicate the deopt.
__DEV__ &&
(recentlyCreatedOwnerStacks++ < ownerStackLimit
? Error('react-stack-top-frame')
: null),
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
__DEV__ &&
(recentlyCreatedDebugTasks++ < debugTaskLimit
? createTask(getTaskName(type))
: null),
);
}

Expand Down Expand Up @@ -700,8 +736,16 @@ export function createElement(type, config, children) {
undefined,
getOwner(),
props,
__DEV__ && Error('react-stack-top-frame'),
__DEV__ && createTask(getTaskName(type)),
// TODO: Reuse a placeholder stack to indicate the deopt.
__DEV__ &&
(recentlyCreatedOwnerStacks++ < ownerStackLimit
? Error('react-stack-top-frame')
: null),
// TODO: If task.run is not the bottleneck, reuse a placeholder task to indicate the deop.
__DEV__ &&
(recentlyCreatedDebugTasks++ < debugTaskLimit
? createTask(getTaskName(type))
: null),
);
}

Expand Down
4 changes: 4 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,7 @@ export const enableUpdaterTracking = __PROFILE__;

// Internal only.
export const enableDO_NOT_USE_disableStrictPassiveEffect = false;

export const ownerStackLimit = 1e4;
export const debugTaskLimit = 1e4;
export const debugInfoLimitsResetIntervalMs = 1000;
9 changes: 9 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,12 @@ export const enableSiblingPrerendering = __VARIANT__;
export const enableUseEffectCRUDOverload = __VARIANT__;
export const enableFastAddPropertiesInDiffing = __VARIANT__;
export const enableLazyPublicInstanceInFabric = __VARIANT__;
export const ownerStackLimit: number = __VARIANT__
? // Some value that doesn't impact existing tests
500
: 1e4;
export const debugTaskLimit: number = __VARIANT__
? // Some value that doesn't impact existing tests
500
: 1e4;
export const debugInfoLimitsResetIntervalMs: number = __VARIANT__ ? 1000 : 2000;
3 changes: 3 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ export const {
enableSiblingPrerendering,
enableFastAddPropertiesInDiffing,
enableLazyPublicInstanceInFabric,
ownerStackLimit,
debugTaskLimit,
debugInfoLimitsResetIntervalMs,
} = dynamicFlags;

// The rest of the flags are static for better dead code elimination.
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ export const enableSwipeTransition = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true;
export const ownerStackLimit = 1e4;
export const debugTaskLimit = 1e4;
export const debugInfoLimitsResetIntervalMs = 1000;

// Profiling Only
export const enableProfilerTimer = __PROFILE__;
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ export const enableSwipeTransition = false;
export const enableFastAddPropertiesInDiffing = true;
export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true;
export const ownerStackLimit = 1e4;
export const debugTaskLimit = 1e4;
export const debugInfoLimitsResetIntervalMs = 1000;

// TODO: This must be in sync with the main ReactFeatureFlags file because
// the Test Renderer's value must be the same as the one used by the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ export const enableSwipeTransition = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true;
export const ownerStackLimit = 1e4;
export const debugTaskLimit = 1e4;
export const debugInfoLimitsResetIntervalMs = 1000;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,9 @@ export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false;
export const enableScrollEndPolyfill = true;

export const ownerStackLimit = 1e4;
export const debugTaskLimit = 1e4;
export const debugInfoLimitsResetIntervalMs = 1000;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
10 changes: 10 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ export const enableLazyPublicInstanceInFabric = false;
export const enableViewTransition = __VARIANT__;
export const enableScrollEndPolyfill = __VARIANT__;

export const ownerStackLimit: number = __VARIANT__
? // Some value that doesn't impact existing tests
500
: 1e4;
export const debugTaskLimit: number = __VARIANT__
? // Some value that doesn't impact existing tests
500
: 1e4;
export const debugInfoLimitsResetIntervalMs: number = __VARIANT__ ? 1000 : 2000;

// TODO: These flags are hard-coded to the default values used in open source.
// Update the tests so that they pass in either mode, then set these
// to __VARIANT__.
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export const {
enableFastAddPropertiesInDiffing,
enableViewTransition,
enableScrollEndPolyfill,
ownerStackLimit,
debugTaskLimit,
debugInfoLimitsResetIntervalMs,
} = dynamicFeatureFlags;

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

0 comments on commit edcfcb8

Please sign in to comment.