Skip to content

Commit

Permalink
Extract loading of turbomodulejsijni into its own function (#38320)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #38320

In this diff I'm extracting the loading of turbomodulejsijni into its own function. This could have perf implications as we are trying to load multimple times the same soLoader

changelog: [intenral] internal

Reviewed By: luluwu2032

Differential Revision: D47409162

fbshipit-source-id: 007e89e99ec0e7fa494d811f9c830b1363acec19
  • Loading branch information
mdvacca authored and facebook-github-bot committed Jul 29, 2023
1 parent 1383a59 commit e924685
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.turbomodule.core.TurboModuleManagerDelegate;
import com.facebook.soloader.SoLoader;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand All @@ -21,8 +20,6 @@
public class CompositeReactPackageTurboModuleManagerDelegate
extends ReactPackageTurboModuleManagerDelegate {

private static volatile boolean sIsSoLibraryLoaded;

protected native HybridData initHybrid();

private CompositeReactPackageTurboModuleManagerDelegate(
Expand Down Expand Up @@ -53,12 +50,4 @@ protected ReactPackageTurboModuleManagerDelegate build(
return new CompositeReactPackageTurboModuleManagerDelegate(context, packages, delegates);
}
}

protected synchronized void maybeLoadOtherSoLibraries() {
// Prevents issues with initializer interruptions. See T38996825 and D13793825 for more context.
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,20 @@

import com.facebook.jni.HybridData;
import com.facebook.react.turbomodule.core.interfaces.CallInvokerHolder;
import com.facebook.soloader.SoLoader;

/**
* JSCallInvoker is created at a different time/place (i.e: in CatalystInstance) than
* TurboModuleManager. Therefore, we need to wrap JSCallInvoker within a hybrid class so that we may
* pass it from CatalystInstance, through Java, to TurboModuleManager::initHybrid.
*/
public class CallInvokerHolderImpl implements CallInvokerHolder {
private static volatile boolean sIsSoLibraryLoaded;

private final HybridData mHybridData;

private CallInvokerHolderImpl(HybridData hd) {
maybeLoadSoLibrary();
mHybridData = hd;
static {
NativeModuleSoLoader.maybeLoadSoLibrary();
}

// Prevents issues with initializer interruptions. See T38996825 and D13793825 for more context.
private static synchronized void maybeLoadSoLibrary() {
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
private CallInvokerHolderImpl(HybridData hd) {
mHybridData = hd;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import com.facebook.jni.HybridData;
import com.facebook.react.turbomodule.core.interfaces.NativeMethodCallInvokerHolder;
import com.facebook.soloader.SoLoader;

/**
* NativeMethodCallInvokerHolder is created at a different time/place (i.e: in CatalystInstance)
Expand All @@ -18,20 +17,14 @@
* TurboModuleManager::initHybrid.
*/
public class NativeMethodCallInvokerHolderImpl implements NativeMethodCallInvokerHolder {
private static volatile boolean sIsSoLibraryLoaded;

private final HybridData mHybridData;

private NativeMethodCallInvokerHolderImpl(HybridData hd) {
maybeLoadSoLibrary();
mHybridData = hd;
static {
NativeModuleSoLoader.maybeLoadSoLibrary();
}

// Prevents issues with initializer interruptions. See T38996825 and D13793825 for more context.
private static synchronized void maybeLoadSoLibrary() {
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
private NativeMethodCallInvokerHolderImpl(HybridData hd) {
mHybridData = hd;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.turbomodule.core

import com.facebook.soloader.SoLoader

internal class NativeModuleSoLoader {
companion object {
private var isSoLibraryLoaded = false

@Synchronized
@JvmStatic
fun maybeLoadSoLibrary() {
if (!isSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni")
isSoLibraryLoaded = true
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.facebook.react.turbomodule.core.interfaces.NativeMethodCallInvokerHolder;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;
import com.facebook.react.turbomodule.core.interfaces.TurboModuleRegistry;
import com.facebook.soloader.SoLoader;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
Expand All @@ -38,12 +37,15 @@
* a Java module, that the C++ counterpart calls.
*/
public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
private static volatile boolean sIsSoLibraryLoaded;
private final List<String> mEagerInitModuleNames;
private final ModuleProvider mTurboModuleProvider;
private final ModuleProvider mLegacyModuleProvider;
private final TurboModuleManagerDelegate mDelegate;

static {
NativeModuleSoLoader.maybeLoadSoLibrary();
}

// Prevents the creation of new TurboModules once cleanup as been initiated.
private final Object mModuleCleanupLock = new Object();

Expand All @@ -63,7 +65,6 @@ public TurboModuleManager(
@Nullable final TurboModuleManagerDelegate delegate,
CallInvokerHolder jsCallInvokerHolder,
NativeMethodCallInvokerHolder nativeMethodCallInvokerHolder) {
maybeLoadSoLibrary();
mDelegate = delegate;
mHybridData =
initHybrid(
Expand Down Expand Up @@ -451,14 +452,6 @@ public void onCatalystInstanceDestroy() {
mHybridData.resetNative();
}

// Prevents issues with initializer interruptions. See T38996825 and D13793825 for more context.
private static synchronized void maybeLoadSoLibrary() {
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
}

private static class ModuleHolder {
private volatile NativeModule mModule = null;
private volatile boolean mIsTryingToCreate = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;
import com.facebook.soloader.SoLoader;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -21,13 +20,14 @@ public abstract class TurboModuleManagerDelegate {
@SuppressWarnings("unused")
private final HybridData mHybridData;

private static volatile boolean sIsSoLibraryLoaded;
static {
NativeModuleSoLoader.maybeLoadSoLibrary();
}

protected abstract HybridData initHybrid();

protected TurboModuleManagerDelegate() {
maybeLoadOtherSoLibraries();
maybeLoadSoLibrary();
mHybridData = initHybrid();
}

Expand Down Expand Up @@ -57,13 +57,5 @@ public List<String> getEagerInitModuleNames() {
return new ArrayList<>();
}

// Prevents issues with initializer interruptions. See T38996825 and D13793825 for more context.
private static synchronized void maybeLoadSoLibrary() {
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
}

protected synchronized void maybeLoadOtherSoLibraries() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@

import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.perflogger.NativeModulePerfLogger;
import com.facebook.soloader.SoLoader;
import javax.annotation.Nullable;

@DoNotStrip
public class TurboModulePerfLogger {

@Nullable private static NativeModulePerfLogger sNativeModulePerfLogger = null;
private static boolean sIsSoLibraryLoaded = false;

static {
NativeModuleSoLoader.maybeLoadSoLibrary();
}

public static void moduleDataCreateStart(String moduleName, int id) {
if (sNativeModulePerfLogger != null) {
Expand Down Expand Up @@ -79,17 +82,9 @@ public static void moduleCreateFail(String moduleName, int id) {

private static native void jniEnableCppLogging(NativeModulePerfLogger perfLogger);

private static synchronized void maybeLoadSoLibrary() {
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
}

public static void enableLogging(NativeModulePerfLogger perfLogger) {
if (perfLogger != null) {
sNativeModulePerfLogger = perfLogger;
maybeLoadSoLibrary();
jniEnableCppLogging(perfLogger);
}
}
Expand Down

0 comments on commit e924685

Please sign in to comment.