Remove some Tasks overhead (facebook#46348)

Summary: Pull Request resolved: facebook#46348 `continueWithTask` skips an extra invocation layer that `onSuccess` adds. Switch to Task<Void> as the task can already represent failure or success without needing a boolean. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D62213722 fbshipit-source-id: 631d741bd2ec4917eab69a20978ab2ace737c459
shwanton · Sep 6, 2024 · 0c90cfc · 0c90cfc
1 parent 3bd3d76
commit 0c90cfc
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 40 deletions.
diff --git a/...ve/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessDevSupportManager.java b/...ve/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessDevSupportManager.java
@@ -23,8 +23,6 @@
 import com.facebook.react.devsupport.ReactInstanceDevHelper;
 import com.facebook.react.devsupport.interfaces.DevSplitBundleCallback;
 import com.facebook.react.modules.core.DeviceEventManagerModule;
-import com.facebook.react.runtime.internal.bolts.Continuation;
-import com.facebook.react.runtime.internal.bolts.Task;
 
 /**
  * An implementation of {@link com.facebook.react.devsupport.interfaces.DevSupportManager} that
@@ -69,20 +67,17 @@ public void onSuccess(final JSBundleLoader bundleLoader) {
             mReactHost
                 .loadBundle(bundleLoader)
                 .onSuccess(
-                    new Continuation<Boolean, Void>() {
-                      @Override
-                      public Void then(Task<Boolean> task) {
-                        if (task.getResult().equals(Boolean.TRUE)) {
-                          String bundleURL =
-                              getDevServerHelper().getDevServerSplitBundleURL(bundlePath);
-                          ReactContext reactContext = mReactHost.getCurrentReactContext();
-                          if (reactContext != null) {
-                            reactContext.getJSModule(HMRClient.class).registerBundle(bundleURL);
-                          }
-                          callback.onSuccess();
+                    task -> {
+                      if (task.isCompleted()) {
+                        String bundleURL =
+                            getDevServerHelper().getDevServerSplitBundleURL(bundlePath);
+                        ReactContext reactContext = mReactHost.getCurrentReactContext();
+                        if (reactContext != null) {
+                          reactContext.getJSModule(HMRClient.class).registerBundle(bundleURL);
                         }
-                        return null;
+                        callback.onSuccess();
                       }
+                      return null;
                     });
           }
 

diff --git a/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java
@@ -9,8 +9,6 @@
 
 import static com.facebook.infer.annotation.Assertions.assertNotNull;
 import static com.facebook.infer.annotation.ThreadConfined.UI;
-import static java.lang.Boolean.FALSE;
-import static java.lang.Boolean.TRUE;
 
 import android.app.Activity;
 import android.content.Context;
@@ -252,13 +250,12 @@ TaskInterface<Void> stopSurface(final ReactSurfaceImpl surface) {
 
     detachSurface(surface);
     return callWithExistingReactInstance(
-            method,
-            reactInstance -> {
-              log(method, "Execute");
-              reactInstance.stopSurface(surface);
-            },
-            mBGExecutor)
-        .makeVoid();
+        method,
+        reactInstance -> {
+          log(method, "Execute");
+          reactInstance.stopSurface(surface);
+        },
+        mBGExecutor);
   }
 
   /**
@@ -779,7 +776,7 @@ DefaultHardwareBackBtnHandler getDefaultBackButtonHandler() {
     };
   }
 
-  /* package */ Task<Boolean> loadBundle(final JSBundleLoader bundleLoader) {
+  /* package */ Task<Void> loadBundle(final JSBundleLoader bundleLoader) {
     final String method = "loadBundle()";
     log(method, "Schedule");
 
@@ -792,7 +789,7 @@ DefaultHardwareBackBtnHandler getDefaultBackButtonHandler() {
         null);
   }
 
-  /* package */ Task<Boolean> registerSegment(
+  /* package */ Task<Void> registerSegment(
       final int segmentId, final String path, final Callback callback) {
     final String method =
         "registerSegment(segmentId = \"" + segmentId + "\", path = \"" + path + "\")";
@@ -828,7 +825,7 @@ DefaultHardwareBackBtnHandler getDefaultBackButtonHandler() {
    * @param args Arguments to be passed to the function
    * @return A Task that will complete when the function call has been enqueued on the JS thread.
    */
-  /* package */ Task<Boolean> callFunctionOnModule(
+  /* package */ Task<Void> callFunctionOnModule(
       final String moduleName, final String methodName, final NativeArray args) {
     final String method = "callFunctionOnModule(\"" + moduleName + "\", \"" + methodName + "\")";
     return callWithExistingReactInstance(
@@ -950,9 +947,9 @@ private Executor getDefaultReactInstanceExecutor() {
   }
 
   /** Schedule work on a ReactInstance that is already created. */
-  private Task<Boolean> callWithExistingReactInstance(
+  private Task<Void> callWithExistingReactInstance(
       final String callingMethod,
-      final ReactInstanceCalback continuation,
+      final ReactInstanceCalback callback,
       @Nullable Executor executor) {
     final String method = "callWithExistingReactInstance(" + callingMethod + ")";
 
@@ -962,27 +959,26 @@ private Task<Boolean> callWithExistingReactInstance(
 
     return mCreateReactInstanceTaskRef
         .get()
-        .onSuccess(
+        .continueWith(
             task -> {
               final ReactInstance reactInstance =
                   ReactNativeFeatureFlags.completeReactInstanceCreationOnBgThreadOnAndroid()
                       ? task.getResult()
                       : mReactInstance;
-              if (reactInstance == null) {
+              if (reactInstance == null || task.isFaulted()) {
                 raiseSoftException(method, "Execute: reactInstance is null. Dropping work.");
-                return FALSE;
+              } else {
+                callback.then(reactInstance);
               }
-
-              continuation.then(reactInstance);
-              return TRUE;
+              return null;
             },
             executor);
   }
 
   /** Create a ReactInstance if it doesn't exist already, and schedule work on it. */
   private Task<Void> callAfterGetOrCreateReactInstance(
       final String callingMethod,
-      final ReactInstanceCalback runnable,
+      final ReactInstanceCalback callback,
       @Nullable Executor executor) {
     final String method = "callAfterGetOrCreateReactInstance(" + callingMethod + ")";
 
@@ -991,18 +987,17 @@ private Task<Void> callAfterGetOrCreateReactInstance(
     }
 
     return getOrCreateReactInstance()
-        .onSuccess(
+        .continueWith(
             task -> {
               final ReactInstance reactInstance =
                   ReactNativeFeatureFlags.completeReactInstanceCreationOnBgThreadOnAndroid()
                       ? task.getResult()
                       : mReactInstance;
-              if (reactInstance == null) {
+              if (reactInstance == null || task.isFaulted()) {
                 raiseSoftException(method, "Execute: reactInstance is null. Dropping work.");
-                return null;
+              } else {
+                callback.then(reactInstance);
               }
-
-              runnable.then(reactInstance);
               return null;
             },
             executor)