From f53dc709747224873a79d2c301af40f73c2729b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 27 Nov 2024 09:11:32 -0800 Subject: [PATCH] Optimize performance of ReactNativeElement constructor (#47983) Summary: Changelog: [internal] This mitigates some performance regressions caused by the migration from `ReactFabricHostComponent` to `ReactNativeElement` (enabling the DOM APIs). Those regressions were caused by 2 main things: 1. By the use of a class hierarchy and having to call `super()`, which we transpile to a very complex code to ensure it's spec compliant. 2. By the use of private fields (`#viewConfig`) which are significantly slower than a regular field with the `_` naming convention (`_viewConfig`) processed by our custom transform. This mitigates those problems by using the `_` convention and refactoring the class hierarchy to avoid the use of `super()` while preserving the Flow typing and most of the existing implementation. Reviewed By: javache, andrewdacenko Differential Revision: D66540756 --- .../webapis/dom/nodes/ReactNativeElement.js | 54 ++++++++++++++++--- .../private/webapis/dom/nodes/ReadOnlyNode.js | 4 +- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/packages/react-native/src/private/webapis/dom/nodes/ReactNativeElement.js b/packages/react-native/src/private/webapis/dom/nodes/ReactNativeElement.js index adecf4042811da..e7850e8bad733b 100644 --- a/packages/react-native/src/private/webapis/dom/nodes/ReactNativeElement.js +++ b/packages/react-native/src/private/webapis/dom/nodes/ReactNativeElement.js @@ -25,7 +25,7 @@ import {getFabricUIManager} from '../../../../../Libraries/ReactNative/FabricUIM import {create as createAttributePayload} from '../../../../../Libraries/ReactNative/ReactFabricPublicInstance/ReactNativeAttributePayload'; import warnForStyleProps from '../../../../../Libraries/ReactNative/ReactFabricPublicInstance/warnForStyleProps'; import ReadOnlyElement, {getBoundingClientRect} from './ReadOnlyElement'; -import ReadOnlyNode from './ReadOnlyNode'; +import ReadOnlyNode, {setInstanceHandle} from './ReadOnlyNode'; import { getPublicInstanceFromInternalInstanceHandle, getShadowNode, @@ -35,7 +35,26 @@ import nullthrows from 'nullthrows'; const noop = () => {}; -export default class ReactNativeElement +// Ideally, this class would be exported as-is, but this implementation is +// significantly slower than the existing `ReactFabricHostComponent`. +// This is a very hot code path (this class is instantiated once per rendered +// host component in the tree) and we can't regress performance here. +// +// This implementation is slower because this is a subclass and we have to call +// super(), which is a very slow operation the way that Babel transforms it at +// the moment. +// +// The optimization we're doing is using an old-style function constructor, +// where we're not required to use `super()`, and we make that constructor +// extend this class so it inherits all the methods and it sets the class +// hierarchy correctly. +// +// An alternative implementation was to implement the constructor as a function +// returning a manually constructed instance using `Object.create()` but that +// was slower than this method because the engine has to create an object than +// we then discard to create a new one. + +class ReactNativeElementMethods extends ReadOnlyElement implements INativeMethods { @@ -43,8 +62,10 @@ export default class ReactNativeElement __nativeTag: number; __internalInstanceHandle: InternalInstanceHandle; - #viewConfig: ViewConfig; + __viewConfig: ViewConfig; + // This constructor isn't really used. See the `ReactNativeElement` function + // below. constructor( tag: number, viewConfig: ViewConfig, @@ -54,7 +75,7 @@ export default class ReactNativeElement this.__nativeTag = tag; this.__internalInstanceHandle = internalInstanceHandle; - this.#viewConfig = viewConfig; + this.__viewConfig = viewConfig; } get offsetHeight(): number { @@ -171,12 +192,12 @@ export default class ReactNativeElement setNativeProps(nativeProps: {...}): void { if (__DEV__) { - warnForStyleProps(nativeProps, this.#viewConfig.validAttributes); + warnForStyleProps(nativeProps, this.__viewConfig.validAttributes); } const updatePayload = createAttributePayload( nativeProps, - this.#viewConfig.validAttributes, + this.__viewConfig.validAttributes, ); const node = getShadowNode(this); @@ -186,3 +207,24 @@ export default class ReactNativeElement } } } + +// Alternative constructor just implemented to provide a better performance than +// calling super() in the original class. +function ReactNativeElement( + this: ReactNativeElementMethods, + tag: number, + viewConfig: ViewConfig, + internalInstanceHandle: InternalInstanceHandle, +) { + this.__nativeTag = tag; + this.__internalInstanceHandle = internalInstanceHandle; + this.__viewConfig = viewConfig; + setInstanceHandle(this, internalInstanceHandle); +} + +ReactNativeElement.prototype = Object.create( + ReactNativeElementMethods.prototype, +); + +// $FlowExpectedError[prop-missing] +export default ReactNativeElement as typeof ReactNativeElementMethods; diff --git a/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js b/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js index e368332a2526f8..39bb71ad00cee8 100644 --- a/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js +++ b/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js @@ -26,6 +26,8 @@ let ReadOnlyElementClass: Class; export default class ReadOnlyNode { constructor(internalInstanceHandle: InternalInstanceHandle) { + // This constructor is inlined in `ReactNativeElement` so if you modify + // this make sure that their implementation stays in sync. setInstanceHandle(this, internalInstanceHandle); } @@ -293,7 +295,7 @@ export function getInstanceHandle(node: ReadOnlyNode): InternalInstanceHandle { return node[INSTANCE_HANDLE_KEY]; } -function setInstanceHandle( +export function setInstanceHandle( node: ReadOnlyNode, instanceHandle: InternalInstanceHandle, ): void {