From 40115d87d45c85a154f1bedfe5c9b00d7899c903 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 17 Dec 2020 17:22:11 -0800 Subject: [PATCH] Roll out package info validation Summary: ## Context Every time we require a NativeModule in Java, we [first try to create it with the TurboModuleManager](https://fburl.com/diffusion/3nkjwea2). In the TurboModule infra, when a NativeModule is requested, [we first create it](https://fburl.com/diffusion/d2c6iout), then [if it's not a TurboModule, we discard the newly created object](https://fburl.com/diffusion/44gjlo6y). This is extremely wasteful, especially when a NativeModule is requested frequently and periodically, like UIManagerModule. Therefore, in D24811838 (https://github.com/facebook/react-native/commit/803a26cb003e6b790e3a1ab31beb0c95795fff0c) fkgozali launched a fix to the infra that would avoid creating the non-TurboModule object in the first place. Today, we're launching this optimization. Reviewed By: fkgozali Differential Revision: D25621570 fbshipit-source-id: dedba4d5ac6fcf2ec3c31e7163a6a226065c708b --- .../react/config/ReactFeatureFlags.java | 5 ---- ...eactPackageTurboModuleManagerDelegate.java | 30 +++++++------------ .../react/uiapp/RNTesterApplication.java | 1 - 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 626a99fccd9..c0e8f35cd00 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -29,11 +29,6 @@ public class ReactFeatureFlags { /** Enable TurboModule JS Codegen. */ public static volatile boolean useTurboModuleJSCodegen = false; - /** - * Enable the fix to validate the TurboReactPackage's module info before resolving a TurboModule. - */ - public static volatile boolean enableTurboModulePackageInfoValidation = false; - /* * This feature flag enables logs for Fabric */ diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/ReactPackageTurboModuleManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/ReactPackageTurboModuleManagerDelegate.java index 74a7ca72d2f..59c2fc9794a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/ReactPackageTurboModuleManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/ReactPackageTurboModuleManagerDelegate.java @@ -15,7 +15,6 @@ import com.facebook.react.TurboReactPackage; import com.facebook.react.bridge.CxxModuleWrapper; import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.ReactApplicationContext; -import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.module.model.ReactModuleInfo; import com.facebook.react.turbomodule.core.interfaces.TurboModule; import java.util.ArrayList; @@ -37,9 +36,7 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule if (reactPackage instanceof TurboReactPackage) { TurboReactPackage pkg = (TurboReactPackage) reactPackage; mPackages.add(pkg); - if (ReactFeatureFlags.enableTurboModulePackageInfoValidation) { - mPackageModuleInfos.put(pkg, pkg.getReactModuleInfoProvider().getReactModuleInfos()); - } + mPackageModuleInfos.put(pkg, pkg.getReactModuleInfoProvider().getReactModuleInfos()); } } } @@ -81,23 +78,16 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule for (final TurboReactPackage pkg : mPackages) { try { - if (ReactFeatureFlags.enableTurboModulePackageInfoValidation) { - final ReactModuleInfo moduleInfo = mPackageModuleInfos.get(pkg).get(moduleName); - if (moduleInfo == null - || !moduleInfo.isTurboModule() - || resolvedModule != null && !moduleInfo.canOverrideExistingModule()) { - continue; - } + final ReactModuleInfo moduleInfo = mPackageModuleInfos.get(pkg).get(moduleName); + if (moduleInfo == null + || !moduleInfo.isTurboModule() + || resolvedModule != null && !moduleInfo.canOverrideExistingModule()) { + continue; + } - final NativeModule module = pkg.getModule(moduleName, mReactApplicationContext); - if (module != null) { - resolvedModule = module; - } - } else { - final NativeModule module = pkg.getModule(moduleName, mReactApplicationContext); - if (resolvedModule == null || module != null && module.canOverrideExistingModule()) { - resolvedModule = module; - } + final NativeModule module = pkg.getModule(moduleName, mReactApplicationContext); + if (module != null) { + resolvedModule = module; } } catch (IllegalArgumentException ex) { /** diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java index 14700729103..b892f527c0e 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -206,7 +206,6 @@ public class RNTesterApplication extends Application implements ReactApplication @Override public void onCreate() { ReactFeatureFlags.useTurboModules = BuildConfig.ENABLE_TURBOMODULE; - ReactFeatureFlags.enableTurboModulePackageInfoValidation = true; ReactFontManager.getInstance().addCustomFont(this, "Rubik", R.font.rubik); super.onCreate(); SoLoader.init(this, /* native exopackage */ false);