From 36688d35e1325737709a446933bf5bc139fea541 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Tue, 14 Apr 2020 18:24:23 -0700 Subject: [PATCH] Remove module cache from ReactPackageTurboModuleManagerDelegate Summary: This cache is unnecessary, because: 1. TurboModuleManager caches all created TurboModules 2. TurboModuleManager calls into the TurboModuleManagerDelegate at most once per NativeModule `moduleName`. This diff also makes ReactPackageTurboModuleManager thread-safe, which should help get rid of the crashes in T46487253. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21027998 fbshipit-source-id: c9b5ccc3da7b81787b749e70aa5e55883317eed7 --- ...eactPackageTurboModuleManagerDelegate.java | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) 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 e8a45458325..65ee5186e47 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 @@ -18,13 +18,10 @@ import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.module.model.ReactModuleInfo; import com.facebook.react.turbomodule.core.interfaces.TurboModule; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModuleManagerDelegate { private final List mPackages = new ArrayList<>(); - private final Map mModules = new HashMap<>(); private final ReactApplicationContext mReactApplicationContext; protected ReactPackageTurboModuleManagerDelegate( @@ -69,11 +66,8 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule return (CxxModuleWrapper) module; } + @Nullable private TurboModule resolveModule(String moduleName) { - if (mModules.containsKey(moduleName)) { - return mModules.get(moduleName); - } - NativeModule resolvedModule = null; for (final TurboReactPackage pkg : mPackages) { @@ -92,20 +86,10 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule } if (resolvedModule instanceof TurboModule) { - mModules.put(moduleName, (TurboModule) resolvedModule); - } else { - /** - * 1. The list of TurboReactPackages doesn't change. 2. TurboReactPackage.getModule is - * deterministic. Therefore, any two invocations of TurboReactPackage.getModule will return - * the same result given that they're provided the same arguments. - * - *

Hence, if module lookup fails once, we know it'll fail every time. Therefore, we can - * write null to the mModules Map and avoid doing this extra work. - */ - mModules.put(moduleName, null); + return (TurboModule) resolvedModule; } - return mModules.get(moduleName); + return null; } @Override