Reducing lock contention in module creation

Reviewed By: javache

Differential Revision: D5147499

fbshipit-source-id: b420cc67ee97fd8cf4bb390098465f968478509f

ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java

@@ -3,10 +3,12 @@
 package com.facebook.react.bridge;
 
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
 import javax.inject.Provider;
 
 import java.util.concurrent.atomic.AtomicInteger;
 
+import com.facebook.infer.annotation.Assertions;
 import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.module.model.ReactModuleInfo;
 import com.facebook.systrace.Systrace;
@@ -36,8 +38,13 @@ public class ModuleHolder {
   private final boolean mHasConstants;
 
   private @Nullable Provider<? extends NativeModule> mProvider;
-  private @Nullable NativeModule mModule;
-  private boolean mInitializeNeeded;
+  // Outside of the constructur, these should only be checked or set when synchronized on this
+  private @Nullable @GuardedBy("this") NativeModule mModule;
+
+  // These are used to communicate phases of creation and initialization across threads
+  private @GuardedBy("this") boolean mInitializable;
+  private @GuardedBy("this") boolean mIsCreating;
+  private @GuardedBy("this") boolean mIsInitializing;
 
   public ModuleHolder(ReactModuleInfo moduleInfo, Provider<? extends NativeModule> provider) {
     mName = moduleInfo.name();
@@ -56,15 +63,28 @@ public class ModuleHolder {
     mModule = nativeModule;
   }
 
-  public synchronized void initialize() {
-    if (mModule != null) {
-      doInitialize(mModule);
-    } else {
-      mInitializeNeeded = true;
+  /*
+  * Checks if mModule has been created, and if so tries to initialize the module unless another
+  * thread is already doing the initialization.
+  * If mModule has not been created, records that initialization is needed
+  */
+  /* package */ void markInitializable() {
+    boolean shouldInitializeNow = false;
+    NativeModule module = null;
+    synchronized (this) {
+      mInitializable = true;
+      if (mModule != null) {
+        Assertions.assertCondition(!mIsInitializing);
+        shouldInitializeNow = true;
+        module = mModule;
+      }
+    }
+    if (shouldInitializeNow) {
+      doInitialize(module);
     }
   }
 
-  public synchronized boolean isInitialized() {
+  /* pacakge */ synchronized boolean hasInstance() {
     return mModule != null;
   }
 
@@ -88,11 +108,44 @@ public class ModuleHolder {
   }
 
   @DoNotStrip
-  public synchronized NativeModule getModule() {
-    if (mModule == null) {
-      mModule = create();
+  public NativeModule getModule() {
+    NativeModule module;
+    boolean shouldCreate = false;
+    synchronized (this) {
+      if (mModule != null) {
+        return mModule;
+      // if mModule has not been set, and no one is creating it. Then this thread should call create
+      } else if (!mIsCreating) {
+        shouldCreate = true;
+        mIsCreating = true;
+      } else  {
+        // Wait for mModule to be created by another thread
+      }
+    }
+    if (shouldCreate) {
+      module = create();
+      // Once module is built (and initialized if markInitializable has been called), modify mModule
+      // And signal any waiting threads that it is acceptable to read the field now
+      synchronized (this) {
+        mIsCreating = false;
+        this.notifyAll();
+      }
+      return module;
+    } else {
+      synchronized (this) {
+        // Block waiting for another thread to build mModule instance
+        // Since mIsCreating is true until after creation and instantiation (if needed), we wait
+        // until the module is ready to use.
+        while (mModule == null && mIsCreating) {
+          try {
+            this.wait();
+          } catch (InterruptedException e) {
+            continue;
+          }
+        }
+        return Assertions.assertNotNull(mModule);
+      }
     }
-    return mModule;
   }
 
   private NativeModule create() {
@@ -106,9 +159,15 @@ public class ModuleHolder {
     try {
       module = assertNotNull(mProvider).get();
       mProvider = null;
-      if (mInitializeNeeded) {
+      boolean shouldInitializeNow = false;
+      synchronized(this) {
+        mModule = module;
+        if (mInitializable && !mIsInitializing) {
+          shouldInitializeNow = true;
+        }
+      }
+      if (shouldInitializeNow) {
         doInitialize(module);
-        mInitializeNeeded = false;
       }
     } finally {
       Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
@@ -123,7 +182,22 @@ public class ModuleHolder {
       .flush();
     ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_START, mName);
     try {
-      module.initialize();
+      boolean shouldInitialize = false;
+      // Check to see if another thread is initializing the object, if not claim the responsibility
+      synchronized (this) {
+        if (mInitializable && !mIsInitializing) {
+          shouldInitialize = true;
+          mIsInitializing = true;
+        }
+      }
+      if (shouldInitialize) {
+        module.initialize();
+        // Once finished, set flags accordingly, but we don't expect anyone to wait for this to finish
+        // So no need to notify other threads
+        synchronized (this) {
+          mIsInitializing = false;
+        }
+      }
     } finally {
       ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_END);
       Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);

ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java

@@ -83,7 +83,7 @@ public class NativeModuleRegistry {
         "NativeModuleRegistry_notifyJSInstanceInitialized");
     try {
       for (ModuleHolder module : mModules.values()) {
-        module.initialize();
+        module.markInitializable();
       }
     } finally {
       Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
@@ -93,7 +93,7 @@ public class NativeModuleRegistry {
 
   public void onBatchComplete() {
     for (ModuleHolder moduleHolder : mBatchCompleteListenerModules) {
-      if (moduleHolder.isInitialized()) {
+      if (moduleHolder.hasInstance()) {
         ((OnBatchCompleteListener) moduleHolder.getModule()).onBatchComplete();
       }
     }