Skip to content

Commit

Permalink
[RN] Set up test to create public instances lazily in Fabric (#32363)
Browse files Browse the repository at this point in the history
## Summary

In React Native, public instances and internal host nodes are not
represented by the same object (ReactNativeElement & shadow nodes vs.
just DOM elements), and the only one that's required for rendering is
the shadow node. Public instances are generally only necessary when
accessed via refs or events, and that usually happens for a small amount
of components in the tree.

This implements an optimization to create the public instance on demand,
instead of eagerly creating it when creating the host node. We expect
this to improve performance by reducing the logic we do per node and the
number of object allocations.

## How did you test this change?

Manually synced the changes to React Native and run Fantom tests and
benchmarks, with the flag enabled and disabled. All tests pass in both
cases, and benchmarks show a slight but consistent performance
improvement.
  • Loading branch information
rubennorte authored Feb 12, 2025
1 parent 192555b commit f83903b
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 19 deletions.
69 changes: 50 additions & 19 deletions packages/react-native-renderer/src/ReactFiberConfigFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ import {
getInspectorDataForInstance,
} from './ReactNativeFiberInspector';

import {passChildrenWhenCloningPersistedNodes} from 'shared/ReactFeatureFlags';
import {
passChildrenWhenCloningPersistedNodes,
enableLazyPublicInstanceInFabric,
} from 'shared/ReactFeatureFlags';
import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
import type {ReactContext} from 'shared/ReactTypes';

Expand Down Expand Up @@ -93,8 +96,11 @@ export type Instance = {
currentProps: Props,
// Reference to the React handle (the fiber)
internalInstanceHandle: InternalInstanceHandle,
// Exposed through refs.
publicInstance: PublicInstance,
// Exposed through refs. Potentially lazily created.
publicInstance: PublicInstance | null,
// This is only necessary to lazily create `publicInstance`.
// Will be set to `null` after that is created.
publicRootInstance?: PublicRootInstance | null,
},
};
export type TextInstance = {
Expand Down Expand Up @@ -186,23 +192,37 @@ export function createInstance(
internalInstanceHandle, // internalInstanceHandle
);

const component = createPublicInstance(
tag,
viewConfig,
internalInstanceHandle,
rootContainerInstance.publicInstance,
);

return {
node: node,
canonical: {
nativeTag: tag,
if (enableLazyPublicInstanceInFabric) {
return {
node: node,
canonical: {
nativeTag: tag,
viewConfig,
currentProps: props,
internalInstanceHandle,
publicInstance: null,
publicRootInstance: rootContainerInstance.publicInstance,
},
};
} else {
const component = createPublicInstance(
tag,
viewConfig,
currentProps: props,
internalInstanceHandle,
publicInstance: component,
},
};
rootContainerInstance.publicInstance,
);

return {
node: node,
canonical: {
nativeTag: tag,
viewConfig,
currentProps: props,
internalInstanceHandle,
publicInstance: component,
},
};
}
}

export function createTextInstance(
Expand Down Expand Up @@ -277,7 +297,18 @@ export function getChildHostContext(
}

export function getPublicInstance(instance: Instance): null | PublicInstance {
if (instance.canonical != null && instance.canonical.publicInstance != null) {
if (instance.canonical != null) {
if (instance.canonical.publicInstance == null) {
instance.canonical.publicInstance = createPublicInstance(
instance.canonical.nativeTag,
instance.canonical.viewConfig,
instance.canonical.internalInstanceHandle,
instance.canonical.publicRootInstance ?? null,
);
// This was only necessary to create the public instance.
instance.canonical.publicRootInstance = null;
}

return instance.canonical.publicInstance;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ export const enableUseEffectCRUDOverload = false;

export const enableFastAddPropertiesInDiffing = true;

export const enableLazyPublicInstanceInFabric = false;

// -----------------------------------------------------------------------------
// Ready for next major.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ export const enableUseEffectCRUDOverload = __VARIANT__;
export const enableOwnerStacks = __VARIANT__;
export const enableRemoveConsolePatches = __VARIANT__;
export const enableFastAddPropertiesInDiffing = __VARIANT__;
export const enableLazyPublicInstanceInFabric = __VARIANT__;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const {
enableOwnerStacks,
enableRemoveConsolePatches,
enableFastAddPropertiesInDiffing,
enableLazyPublicInstanceInFabric,
} = dynamicFlags;

// The rest of the flags are static for better dead code elimination.
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const enableYieldingBeforePassive = false;
export const enableThrottledScheduling = false;
export const enableViewTransition = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false;

// Profiling Only
export const enableProfilerTimer = __PROFILE__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export const enableYieldingBeforePassive = true;
export const enableThrottledScheduling = false;
export const enableViewTransition = false;
export const enableFastAddPropertiesInDiffing = true;
export const enableLazyPublicInstanceInFabric = false;

// 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 @@ -69,6 +69,7 @@ export const enableThrottledScheduling = false;
export const enableViewTransition = false;
export const enableRemoveConsolePatches = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export const enableThrottledScheduling = false;
export const enableViewTransition = false;
export const enableRemoveConsolePatches = false;
export const enableFastAddPropertiesInDiffing = false;
export const enableLazyPublicInstanceInFabric = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const enableSiblingPrerendering = __VARIANT__;
export const enableUseEffectCRUDOverload = __VARIANT__;
export const enableRemoveConsolePatches = __VARIANT__;
export const enableFastAddPropertiesInDiffing = __VARIANT__;
export const enableLazyPublicInstanceInFabric = false;
export const enableViewTransition = __VARIANT__;

// TODO: These flags are hard-coded to the default values used in open source.
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,7 @@ export const disableLegacyMode = true;

export const enableShallowPropDiffing = false;

export const enableLazyPublicInstanceInFabric = false;

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

0 comments on commit f83903b

Please sign in to comment.