Skip to content

Commit

Permalink
Roll out package info validation
Browse files Browse the repository at this point in the history
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 (803a26c) 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
  • Loading branch information
RSNara authored and facebook-github-bot committed Dec 18, 2020
1 parent 5a37773 commit 40115d8
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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;
Expand All @@ -37,9 +36,7 @@ protected ReactPackageTurboModuleManagerDelegate(
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());
}
}
}
Expand Down Expand Up @@ -81,23 +78,16 @@ private TurboModule resolveModule(String moduleName) {

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 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 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;
}
} catch (IllegalArgumentException ex) {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ public double getDouble(final String s) {
@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);
Expand Down

0 comments on commit 40115d8

Please sign in to comment.