Skip to content

Commit

Permalink
Remove some Tasks overhead (facebook#46348)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
javache authored and facebook-github-bot committed Sep 6, 2024
1 parent 3bd3d76 commit 0c90cfc
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
});
}

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

/**
Expand Down Expand Up @@ -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");

Expand All @@ -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 + "\")";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 + ")";

Expand All @@ -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 + ")";

Expand All @@ -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)
Expand Down

0 comments on commit 0c90cfc

Please sign in to comment.