stop sending State in view preallocation on Android

Summary: changelog: [internal] state object is usually invalid when first constructed and with view preallocation, it remains unused. This is unnecessary work that we can avoid by simply not sending state through jni during view preallocation. Differential Revision: D58529357
facebook · Jun 14, 2024 · 22a1813 · 22a1813
1 parent 43d0383
commit 22a1813
Show file tree

Hide file tree

Showing 6 changed files with 7 additions and 35 deletions.
diff --git a/...es/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/...es/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@@ -74,7 +74,6 @@
 import com.facebook.react.uimanager.ReactRoot;
 import com.facebook.react.uimanager.ReactRootViewTagGenerator;
 import com.facebook.react.uimanager.RootViewUtil;
-import com.facebook.react.uimanager.StateWrapper;
 import com.facebook.react.uimanager.ThemedReactContext;
 import com.facebook.react.uimanager.UIManagerHelper;
 import com.facebook.react.uimanager.ViewManagerPropertyUpdater;
@@ -749,7 +748,6 @@ private void preallocateView(
       int reactTag,
       final String componentName,
       @Nullable Object props,
-      @Nullable Object stateWrapper,
       @Nullable Object eventEmitterWrapper,
       boolean isLayoutable) {
 
@@ -759,7 +757,6 @@ private void preallocateView(
             reactTag,
             componentName,
             (ReadableMap) props,
-            (StateWrapper) stateWrapper,
             (EventEmitterWrapper) eventEmitterWrapper,
             isLayoutable));
   }

diff --git a/...ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/...ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
@@ -1208,7 +1208,6 @@ public void preallocateView(
       @NonNull String componentName,
       int reactTag,
       @Nullable ReadableMap props,
-      @Nullable StateWrapper stateWrapper,
       @Nullable EventEmitterWrapper eventEmitterWrapper,
       boolean isLayoutable) {
     UiThreadUtil.assertOnUiThread();
@@ -1222,8 +1221,7 @@ public void preallocateView(
       return;
     }
 
-    createViewUnsafe(
-        componentName, reactTag, props, stateWrapper, eventEmitterWrapper, isLayoutable);
+    createViewUnsafe(componentName, reactTag, props, null, eventEmitterWrapper, isLayoutable);
   }
 
   @AnyThread

diff --git a/...ctAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/MountItemFactory.kt b/...ctAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/MountItemFactory.kt
@@ -10,7 +10,6 @@ package com.facebook.react.fabric.mounting.mountitems
 import com.facebook.react.bridge.ReadableArray
 import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.fabric.events.EventEmitterWrapper
-import com.facebook.react.uimanager.StateWrapper
 
 /** Factory class that expose creation of [MountItem] */
 public object MountItemFactory {
@@ -49,12 +48,11 @@ public object MountItemFactory {
       reactTag: Int,
       component: String,
       props: ReadableMap?,
-      stateWrapper: StateWrapper?,
       eventEmitterWrapper: EventEmitterWrapper?,
       isLayoutable: Boolean
   ): MountItem =
       PreAllocateViewMountItem(
-          surfaceId, reactTag, component, props, stateWrapper, eventEmitterWrapper, isLayoutable)
+          surfaceId, reactTag, component, props, eventEmitterWrapper, isLayoutable)
 
   /**
    * @return a [MountItem] that will be read and execute a collection of MountItems serialized in

diff --git a/...src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java b/...src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java
@@ -19,7 +19,6 @@
 import com.facebook.react.fabric.events.EventEmitterWrapper;
 import com.facebook.react.fabric.mounting.MountingManager;
 import com.facebook.react.fabric.mounting.SurfaceMountingManager;
-import com.facebook.react.uimanager.StateWrapper;
 
 /** {@link MountItem} that is used to pre-allocate views for JS components. */
 @Nullsafe(Nullsafe.Mode.LOCAL)
@@ -29,7 +28,6 @@ final class PreAllocateViewMountItem implements MountItem {
   private final int mSurfaceId;
   private final int mReactTag;
   private final @Nullable ReadableMap mProps;
-  private final @Nullable StateWrapper mStateWrapper;
   private final @Nullable EventEmitterWrapper mEventEmitterWrapper;
   private final boolean mIsLayoutable;
 
@@ -38,13 +36,11 @@ final class PreAllocateViewMountItem implements MountItem {
       int reactTag,
       @NonNull String component,
       @Nullable ReadableMap props,
-      @Nullable StateWrapper stateWrapper,
       @Nullable EventEmitterWrapper eventEmitterWrapper,
       boolean isLayoutable) {
     mComponent = getFabricComponentName(component);
     mSurfaceId = surfaceId;
     mProps = props;
-    mStateWrapper = stateWrapper;
     mEventEmitterWrapper = eventEmitterWrapper;
     mReactTag = reactTag;
     mIsLayoutable = isLayoutable;
@@ -65,7 +61,7 @@ public void execute(@NonNull MountingManager mountingManager) {
       return;
     }
     surfaceMountingManager.preallocateView(
-        mComponent, mReactTag, mProps, mStateWrapper, mEventEmitterWrapper, mIsLayoutable);
+        mComponent, mReactTag, mProps, mEventEmitterWrapper, mIsLayoutable);
   }
 
   @Override
@@ -82,11 +78,7 @@ public String toString() {
             .append(mIsLayoutable);
 
     if (IS_DEVELOPMENT_ENVIRONMENT) {
-      result
-          .append(" props: ")
-          .append(mProps != null ? mProps.toString() : "<null>")
-          .append(" state: ")
-          .append(mStateWrapper != null ? mStateWrapper.toString() : "<null>");
+      result.append(" props: ").append(mProps != null ? mProps.toString() : "<null>");
     }
 
     return result.toString();

diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
@@ -801,23 +801,12 @@ void FabricMountingManager::preallocateShadowView(
 
   static auto preallocateView =
       JFabricUIManager::javaClassStatic()
-          ->getMethod<void(
-              jint, jint, jstring, jobject, jobject, jobject, jboolean)>(
+          ->getMethod<void(jint, jint, jstring, jobject, jobject, jboolean)>(
               "preallocateView");
 
-  // Do not hold onto Java object from C
-  // We DO want to hold onto C object from Java, since we don't know the
-  // lifetime of the Java object
-  jni::local_ref<StateWrapperImpl::JavaPart> javaStateWrapper = nullptr;
-  if (shadowView.state != nullptr) {
-    javaStateWrapper = StateWrapperImpl::newObjectJavaArgs();
-    StateWrapperImpl* cStateWrapper = cthis(javaStateWrapper);
-    cStateWrapper->state_ = shadowView.state;
-  }
-
   // Do not hold a reference to javaEventEmitter from the C++ side.
-  auto javaEventEmitter = EventEmitterWrapper::newObjectCxxArgs(
-          shadowView.eventEmitter);
+  auto javaEventEmitter =
+      EventEmitterWrapper::newObjectCxxArgs(shadowView.eventEmitter);
 
   jni::local_ref<jobject> props = getProps({}, shadowView);
 
@@ -829,7 +818,6 @@ void FabricMountingManager::preallocateShadowView(
       shadowView.tag,
       component.get(),
       props.get(),
-      (javaStateWrapper != nullptr ? javaStateWrapper.get() : nullptr),
       javaEventEmitter.get(),
       isLayoutableShadowNode);
 }

diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/ViewShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/ViewShadowNode.cpp
@@ -9,7 +9,6 @@
 #include <react/config/ReactNativeConfig.h>
 #include <react/renderer/components/view/HostPlatformViewTraitsInitializer.h>
 #include <react/renderer/components/view/primitives.h>
-#include <react/utils/CoreFeatures.h>
 
 namespace facebook::react {