Skip to content

Commit

Permalink
stop sending State in view preallocation on Android (#44954)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #44954

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.

Reviewed By: alanleedev

Differential Revision: D58529357
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Jul 9, 2024
1 parent 994fbf3 commit dab68b6
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -749,17 +748,11 @@ private void preallocateView(
int reactTag,
final String componentName,
@Nullable Object props,
@Nullable Object stateWrapper,
boolean isLayoutable) {

mMountItemDispatcher.addPreAllocateMountItem(
MountItemFactory.createPreAllocateViewMountItem(
rootTag,
reactTag,
componentName,
(ReadableMap) props,
(StateWrapper) stateWrapper,
isLayoutable));
rootTag, reactTag, componentName, (ReadableMap) props, isLayoutable));
}

@SuppressLint("NotInvokedPrivateMethod")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,6 @@ public void preallocateView(
@NonNull String componentName,
int reactTag,
@Nullable ReadableMap props,
@Nullable StateWrapper stateWrapper,
boolean isLayoutable) {
UiThreadUtil.assertOnUiThread();

Expand All @@ -1250,7 +1249,7 @@ public void preallocateView(
return;
}

createViewUnsafe(componentName, reactTag, props, stateWrapper, null, isLayoutable);
createViewUnsafe(componentName, reactTag, props, null, null, isLayoutable);
}

@AnyThread
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ package com.facebook.react.fabric.mounting.mountitems

import com.facebook.react.bridge.ReadableArray
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.uimanager.StateWrapper

/** Factory class that expose creation of [MountItem] */
public object MountItemFactory {
Expand Down Expand Up @@ -48,10 +47,8 @@ public object MountItemFactory {
reactTag: Int,
component: String,
props: ReadableMap?,
stateWrapper: StateWrapper?,
isLayoutable: Boolean
): MountItem =
PreAllocateViewMountItem(surfaceId, reactTag, component, props, stateWrapper, isLayoutable)
): MountItem = PreAllocateViewMountItem(surfaceId, reactTag, component, props, isLayoutable)

/**
* @return a [MountItem] that will be read and execute a collection of MountItems serialized in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.facebook.react.bridge.ReadableMap;
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)
Expand All @@ -28,20 +27,17 @@ 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 boolean mIsLayoutable;

PreAllocateViewMountItem(
int surfaceId,
int reactTag,
@NonNull String component,
@Nullable ReadableMap props,
@Nullable StateWrapper stateWrapper,
boolean isLayoutable) {
mComponent = getFabricComponentName(component);
mSurfaceId = surfaceId;
mProps = props;
mStateWrapper = stateWrapper;
mReactTag = reactTag;
mIsLayoutable = isLayoutable;
}
Expand All @@ -60,8 +56,7 @@ public void execute(@NonNull MountingManager mountingManager) {
"Skipping View PreAllocation; no SurfaceMountingManager found for [" + mSurfaceId + "]");
return;
}
surfaceMountingManager.preallocateView(
mComponent, mReactTag, mProps, mStateWrapper, mIsLayoutable);
surfaceMountingManager.preallocateView(mComponent, mReactTag, mProps, mIsLayoutable);
}

@Override
Expand All @@ -78,11 +73,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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ void FabricMountingManager::maybePreallocateShadowView(

static auto preallocateView =
JFabricUIManager::javaClassStatic()
->getMethod<void(jint, jint, jstring, jobject, jobject, jboolean)>(
->getMethod<void(jint, jint, jstring, jobject, jboolean)>(
"preallocateView");

// Do not hold onto Java object from C
Expand All @@ -840,7 +840,6 @@ void FabricMountingManager::maybePreallocateShadowView(
shadowView.tag,
component.get(),
props.get(),
(javaStateWrapper != nullptr ? javaStateWrapper.get() : nullptr),
isLayoutableShadowNode);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down

0 comments on commit dab68b6

Please sign in to comment.