Commit Graph
99 Commits
Author SHA1 Message Date
Ramanpreet NaraandFacebook GitHub Bot 034c6dfe34 Stop sharing LongLivedObjectCollection with the bridge
Summary:
This is the Android analogue to D30019833.

Changelog: [Internal]

Reviewed By: p-sun

Differential Revision: D30029295

fbshipit-source-id: 13df0dfb915697eeedcc527dcdb6c246e89afb0c
2021-08-02 11:22:22 -07:00
Ramanpreet NaraandFacebook GitHub Bot 1e4dd5e877 Clean up RAIICallbackManager experiment
Summary:
This experiment was shipped in D27436402 (https://github.com/facebook/react-native/commit/3d1afbbda301d48a75e45f73b96cd51ae5105dd8).

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D30023039

fbshipit-source-id: 5f7335f2ddaf6f4e2d876a917aaff2cf3d906b5c
2021-08-02 11:22:22 -07:00
Joshua GrossandFacebook GitHub Bot 3d1afbbda3 destroy callbacks even if they aren't called, when java object is destroyed
Summary:
JSI callbacks are only destroyed if the callback is called. If the callback is never called, we're potentially leaking a lot of callbacks.

To mitigate this, we add a wrapper object that is owned by the std::function. Whenever the std::function is destroyed, the wrapper is destroyed and it deallocates the callback as well.

Changelog: [Internal]

Reviewed By: RSNara

Differential Revision: D27436402

fbshipit-source-id: d153640d5d7988c7fadaf2cb332ec00dadd0689a
2021-04-01 16:28:26 -07:00
Samuel SuslaandFacebook GitHub Bot 9ef995a14a Enable C++ 17 in React Native
Summary:
Changelog: [Internal] enable support for C++ 17.

C++ 17 in React Native targets.

Short and comprehensive list of C++ features:
https://github.com/AnthonyCalandra/modern-cpp-features#c17-language-features

Reviewed By: JoshuaGross

Differential Revision: D27431145

fbshipit-source-id: e8da6fe9d70e9b7343a8caec21cdbeb043478575
2021-04-01 04:50:59 -07:00
Ramanpreet NaraandFacebook GitHub Bot 2b2c69aab1 Prevent TurboModuleManagerDelegate.mHybridData from being dead-code eliminated
Summary:
mHybridData should have a SuppressWarnings("unsed") annotation on it. Furthermore, we should add a DoNotStrip annotation, to make sure that the HybridData object doesn't get removed. We do the same in TurboModuleManager:

https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java?commit=5e1cd0c2ed84ff8e718473ff26145733868ebaba&lines=46-48

Changelog: [Internal]

Created from Diffusion's 'Open in Editor' feature.

Reviewed By: fkgozali

Differential Revision: D26958686

fbshipit-source-id: 83c0626c0681966dfcda7f6e4fb4cbeac8340b3f
2021-03-10 15:10:51 -08:00
Ramanpreet NaraandFacebook GitHub Bot c94ef1e3b3 Introduce TurboModule.initialize()
Summary:
NativeModules have an initialize() method that they use to allocate any resources, set up listeners, etc. This diff imports that method into the TurboModule interface. This way, we don't have to cast TurboModules to NativeModules to initialize them. Also, it makes sense to import this initialization mechanism into the TurboModule infra.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D26871552

fbshipit-source-id: b8ae515b22928ed678b4003096e0756e991e10ff
2021-03-06 20:30:18 -08:00
Ramanpreet NaraandFacebook GitHub Bot 18c8417290 Deprecate NativeModule.onCatalystInstanceDestroy()
Summary:
## Rationale
The CatalystInstance is going away after we delete the bridge. So, we should migrate away from onCatalystInstanceDestroy() to something else: invalidate().

## Changes
- Introduce the NativeModule.invalidate() cleanup hook.
- Both the NativeModule and TurboModule infra now call this invalidate() method, **as opposed to** onCatalystInstanceDestroy(), to perform NativeModule cleanup.
- **Is this safe?** All our NativeModules extend BaseJavaModule. BaseJavaModule.invalidate() delegates to NativeModule.onCatalystInstanceDestroy(), so NativeModules that implement onCatalystInstanceDestroy(), but not invalidate(), still have their onCatalystInstanceDestroy() method called.

Changelog: [Android][Deprecated] - Deprecate NativeModule.onCatalystInstanceDestroy() for NativeModule.invalidate()

Reviewed By: JoshuaGross

Differential Revision: D26871001

fbshipit-source-id: e3bdfa0cf653ecbfe42791631bc6229af62f4817
2021-03-06 20:30:17 -08:00
Ramanpreet NaraandFacebook GitHub Bot 2a1b5193f7 Allow ReactInstanceManager to create TurboModuleManager
Summary:
Previously, owner(TurboModuleManager) used to depend on owner(ReactInstanceManager). Now, owner(ReactInstanceManager) depends on owner(TurboModuleManager).

**Rationale:** This allows ReactInstanceManager to create TurboModuleManager.

## Changes
We moved ReactPackageTurboModuleManagerDelegate to com.facebook.react.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D26163661

fbshipit-source-id: 3ebff16ef2aa77e20bb55500ed44c9214acb91dd
2021-02-02 20:50:30 -08:00
generatedunixname89002005325676andFacebook GitHub Bot aebccd3f92 Daily arc lint --take CLANGFORMAT
Reviewed By: zertosh

Differential Revision: D25921551

fbshipit-source-id: df0445864751c18eaa240deff6a142dd791d32ff
2021-01-15 04:16:22 -08:00
generatedunixname89002005325674andFacebook GitHub Bot 0936e7fa9e Daily arc lint --take GOOGLEJAVAFORMAT
Reviewed By: zertosh

Differential Revision: D25921598

fbshipit-source-id: f82ab6a1a891dd6262bfa516f869850415ce3228
2021-01-15 04:12:28 -08:00
Ramanpreet NaraandFacebook GitHub Bot a156ee9b73 Delete JS TurboModule Codegen Gating
Summary: Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D25915171

fbshipit-source-id: b59e21f834a7172055e180eddb9bf15737a6cf0f
2021-01-14 19:14:23 -08:00
Ramanpreet NaraandFacebook GitHub Bot 8ed6659907 Stop forwarding TurboModule schema to TurboModule HostObjects
Summary: Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D25915170

fbshipit-source-id: 2b390428c1f582e55cf3ffe8e691f069bf2fd295
2021-01-14 19:14:23 -08:00
Ramanpreet NaraandFacebook GitHub Bot 5c4f145e33 Roll out TurboModule Promise Async Dispatch
Summary:
## Context
The legacy NativeModule infra implements promise methods using [async NativeModule method calls](https://fburl.com/diffusion/tpkff6vg). In the TurboModule infra, promise methods [treat as sync methods](https://fburl.com/diffusion/yde7xw71), and executed directly on the JavaScript thread. This experiment makes TurboModule promise methods async, and dispatches them to the NativeModules thread, when they're executed from JavaScript.

Reviewed By: fkgozali

Differential Revision: D25623192

fbshipit-source-id: 2b50d771c5272af3b6edf150054bb3e80cab0040
2020-12-17 17:27:09 -08:00
Ramanpreet NaraandFacebook GitHub Bot 40115d87d4 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
2020-12-17 17:27:09 -08:00
Ron EdelsteinandFacebook GitHub Bot 933cef6d9a More explicit marking of autoglob as False
Summary:
In preparation for flipping the default, marking autoglob as False in places where it isn't explicitly specified.

Changelog: [Internal]

Reviewed By: strulovich

Differential Revision: D25497305

fbshipit-source-id: 142e5caca2d67efcb3c25067a36934f7f6dd4b21
2020-12-11 16:45:55 -08:00
Ramanpreet NaraandFacebook GitHub Bot 374808f307 Delete redundant dependency in turbomodules/core:coreAndroid
Reviewed By: ejanzer

Differential Revision: D25274090

fbshipit-source-id: 6f18ebee410b0866d5c5f96cc80631116f4251ff
2020-12-04 12:00:01 -08:00
Xiaoyu YinandFacebook GitHub Bot b9b2215174 Add DoNotStrip annotations to TurboModulePerfLogger
Summary:
Prevent proguard from stripping these methods

Changelog: [Internal] Add DoNotStrip annotations to TurboModulePerfLogger

Reviewed By: RSNara

Differential Revision: D25101027

fbshipit-source-id: 673e97446a36fe0e8c10916aa7eb4c39c2187ba8
2020-11-30 14:15:24 -08:00
Emily JanzerandFacebook GitHub Bot c0a2998387 Move TurboModuleManager to use RuntimeExecutor instead of jsContext (#30416)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/30416

This diff changes the constructor param for TurboModuleManager from jsContext (a long representing the `jsi::Runtime` pointer) to a RuntimeExecutor. It also updates callsites to use the new RuntimeExecutor created by CatalystInstance. This is only used for installing the TurboModule JSI binding; it's not currently used for JS invocation in TurboModules, which is handled separately by JSCallInvoker. Ultimately we may be able to implement JSCallInvoker *with* the provided RuntimeExecutor, but there's some additional logic in JSCallInvoker that we don't have here yet.

Changelog: [Internal]

Reviewed By: RSNara

Differential Revision: D21338930

fbshipit-source-id: 1480c328f1a1776ddf22752510c0f3b35168a489
2020-11-20 14:32:59 -08:00
Kevin GozaliandFacebook GitHub Bot 803a26cb00 TM Android: Avoid creating TM instance if the module is not TM enabled
Summary:
There is a flow where TM registry is creating a module instance (as registered in the TurboReactPackage), only to discard it if it's not a TM enabled module. This may be fine for many modules, but for module like `UIManagerModule`, this may cause a race condition or other issues, including potential perf regression when accessing UIManager from JS (e.g. for getting native viewConfigs).

Changelog: [Internal]

Reviewed By: RSNara

Differential Revision: D24811838

fbshipit-source-id: 6e1cce6993a6e5c9763773f175083bf52925c910
2020-11-09 02:24:41 -08:00
Ramanpreet NaraandFacebook GitHub Bot 610dcf488b Implement method dispatch using TurboModuleSchema
Summary:
This is the final diff in the JS TurboModule codegen stack for Android. It implements method dispatch using the TurboModuleSchema object.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D22837486

fbshipit-source-id: f91b03f064941457d4b8c5e37e011468559dee71
2020-11-08 14:24:05 -08:00
Ramanpreet NaraandFacebook GitHub Bot cb7f3f4499 Setup TurboModule JS Codegen experiment
Summary:
## Android API
```
// Before we initialize TurboModuleManager
ReactFeatureFlags.useTurboModuleJSCodegen = true
```

## iOS API
```
// Before we initialize RCTBridge
RCTEnableTurboModuleJSCodegen(true);
```

## How is the JS Codegen actually enabled?
The above native flags are translated to the following global variable in JavaScript:
```
global.RN$JSTurboModuleCodegenEnabled = true;
```

Then, all our NativeModule specs are transpiled to contain this logic:
```
interface Foo extends TurboModule {
  // ...
}

function __getModuleSchema() {
  if (!global.RN$JSTurboModuleCodegenEnabled) {
    return undefined;
  }

  // Return the schema of this spec.
  return {...};
}

export default TurboModuleRegistry.get<Foo>('foo', __getModuleSchema());
```

Then, in our C++ JavaTurboModule, and ObjCTurboModule classes, we use the TurboModule JS codegen when the jsi::Object schema is provided from JavaScript in the TurboModuleRegistry.get call.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D24636307

fbshipit-source-id: 80dcd604cc1121b8a69df875bbfc87e9bb8e4814
2020-11-06 13:28:15 -08:00
Ramanpreet NaraandFacebook GitHub Bot f1e292b9c1 Set up experiment to dispatch promise methods asynchronously to NativeModules thread
Summary:
TurboModule methods that return promises are synchronously run on the JavaScript thread. Back in D22489338 (https://github.com/facebook/react-native/commit/9c35b5b8c4710dfe6a4b689a5565aa78ae5b37d3), we wrote code to make them dispatch on the NativeModules thread. That code, however, was just left disabled. In this diff, I wire up the TurboModules infra to a MobileConfig, which should allow us to assess the performance impact of async dispatch of promise methods to the NativeModules thread in production, before we roll it out more widely.

Changelog: [Internal]

NOTE: This diff was reverted, beacuse we landed it it without D24685387.

Reviewed By: ejanzer

Differential Revision: D24787573

fbshipit-source-id: 324bd22ce79c2c16c7f7b6996496d255a2c6256e
2020-11-06 13:28:15 -08:00
Alessandro SistoandFacebook GitHub Bot 2af406b88d Revert D24685389: Set up experiment to dispatch promise methods asynchronously to NativeModules thread
Differential Revision:
D24685389 (https://github.com/facebook/react-native/commit/052e578ed666f2749ec06d1d928bb622cada8841)

Original commit changeset: 8ceb2e6effc1

fbshipit-source-id: e48186dd0e928fe95cc7f2420fc02c00a62b8360
2020-11-06 06:26:29 -08:00
Ramanpreet NaraandFacebook GitHub Bot 052e578ed6 Set up experiment to dispatch promise methods asynchronously to NativeModules thread
Summary:
TurboModule methods that return promises are synchronously run on the JavaScript thread. Back in D22489338 (https://github.com/facebook/react-native/commit/9c35b5b8c4710dfe6a4b689a5565aa78ae5b37d3), we wrote code to make them dispatch on the NativeModules thread. That code, however, was just left disabled. In this diff, I wire up the TurboModules infra to a MobileConfig, which should allow us to assess the performance impact of async dispatch of promise methods to the NativeModules thread in production, before we roll it out more widely.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D24685389

fbshipit-source-id: 8ceb2e6effc125abecfa76e5e90bd310676aefc9
2020-11-06 01:24:25 -08:00
Kevin GozaliandFacebook GitHub Bot 7cfc7d65f7 Codegen: Make react-native-codegen BUCK deps OSS-compatible
Summary:
Added a few FB vs OSS polyfills:
* react_native_root_target() to refer to the root FB react-native-github/ dir or repo dir in OSS
* react_native_xplat_synced_target() for anything xplat
* a few others

Changelog: [Internal]

Reviewed By: yungsters

Differential Revision: D24437245

fbshipit-source-id: ee290a87a98a8e9be67b102a96f2faac2a2cb92b
2020-10-22 17:09:29 -07:00
Kevin GozaliandFacebook GitHub Bot 4072b1865a TurboModule Android: rename libreact_nativemodule_manager to libturbomodulejsijni
Summary:
TurboModule Java files are still using the old lib name: `turbomodulejsijni`, so let's keep it that way for now.

Changelog: [Internal]

Reviewed By: yungsters

Differential Revision: D23946945

fbshipit-source-id: ff095ff51dca532c82e67e1c75e9a4e9be392d61
2020-09-25 22:14:56 -07:00
Kevin GozaliandFacebook GitHub Bot 5be44456f2 TurboModule Android: compile TurboModule C++ Core into ReactAndroid
Summary:
This is to prepare for enabling TurboModule on Android. This commit compiles in all the core files (C++) into the ReactAndroid NDK build step. This doesn't yet enable TurboModule by default, just compiling in the infra, just like for iOS.

New shared libs:
* libreact_nativemodule_core.so: The TurboModule Android core
* libreact_nativemodule_manager.so: The TurboModule manager/delegate

To be compatible with `<ReactCommon/` .h include prefix, the files had to move to local `ReactCommon` subdirs.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D23805717

fbshipit-source-id: b41c392a592dd095ae003f7b2a689f4add2c37a9
2020-09-20 14:23:43 -07:00
Kevin GozaliandFacebook GitHub Bot 7c93f5b001 Move TurboModule Core from ReactCommon/turbomodule to ReactCommon/react/nativemodule
Summary:
This diff moves the code of TurboModule Core from ReactCommon/turbomodule to ReactCommon/react/nativemodule

For iOS: Pod spec name stays as "ReactCommon/turbomodule/..." for now, only the source/header location is affected. The target will be renamed/restructured closer to TurboModule rollout.

changelog: [internal] Internal

Reviewed By: RSNara

Differential Revision: D23362253

fbshipit-source-id: c2c8207578e50821c7573255d4319b9051b58a37
2020-09-19 00:42:30 -07:00
Ramanpreet NaraandFacebook GitHub Bot 8a8c15f9fa Remove log when TurboModule cannot be created
Summary:
This log was necessary to get to the bottom of the TurboModule Fb4a eager init crash. It's no longer necessary, plus it's okay for TurboModules to be null, so we'll remove it for now.

Changelog: [Internal]

Differential Revision: D23607208

fbshipit-source-id: 083b00abe6bdefc5986f842cd6f9438f47cce1ce
2020-09-09 16:03:16 -07:00
Ramanpreet NaraandFacebook GitHub Bot 3f77367883 Delay turbomodulejsijni so load until we need it in TurboModulePerfLogger
Summary:
Twilight doesn't have TMPerfLogging enabled. However, the TurboModule infra uses the TMPerfLogger java class everywhere, which loads the turbomodulejsijni library on class load. For some reason, this class load doesn't work, and causes Twilight prod to crash.

To mitigate that crash, this diff delays the so load until it's absolutely necessary, which is by the time we call jniEnableCppLogging. This should never be called in Twilight, because it doesn't have TMPerfLogging enabled. Therefore, the crash should disappear on Twilight.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D23192072

fbshipit-source-id: b73ece580e4345dbf835b0fc2f7d43b90f202411
2020-08-18 12:13:35 -07:00
David VaccaandFacebook GitHub Bot d39fe0fbf9 Remove fb/xplat_init dependency
Summary:
This diff removes the fb/xplat_init dependency from fabric onLoad class
This is necessary to make RM compile in OSS
changelog: [Internal] Internal

Reviewed By: RSNara

Differential Revision: D22875531

fbshipit-source-id: cc4cd2af875fe7eadfb3a8f4a9f16acf5fa415d8
2020-08-01 13:31:03 -07:00
Ramanpreet NaraandFacebook GitHub Bot 111aab590e Forward the NativeModule schema jsi::Value to TMM
Summary:
If `__turboModuleProxy` is called with a second argument, we'll now forward that `jsi::Value` to TurboModuleManager on iOS and Android, so that the TurboModuleManager can eventually use this `jsi::Value` to read data required to perform method invocation on the TurboModule object.

**Note:** This diff is basically a no-op right now.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D22828838

fbshipit-source-id: 19db2adcae6a58b4885fcd11bef23f9d5882bfce
2020-07-31 15:29:49 -07:00
Ramanpreet NaraandFacebook GitHub Bot 826067736f Instrument TurboModuleManager.getModule
Summary:
This instruments the following marker:
- MODULE_CREATE

**Note:** This marker isn't necessary to test the JS TurboModule codegen, since the JS codegen should only affect the C++ portion of the TurboModule infra. However, I implemented this while I was in this area of the code, for completeness.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D22679888

fbshipit-source-id: aa04822bd5a7c889813fcd13ca23c0b7a1d8444a
2020-07-31 12:49:15 -07:00
Ramanpreet NaraandFacebook GitHub Bot 9c35b5b8c4 Dispatch promise methods to the NativeModules thread
Summary:
In D17480605 (https://github.com/facebook/react-native/commit/689233b018bd533a7eecd38e38a7fb84b849cf88), I made all methods with void return types dispatch to the NativeModules thread. This diff makes the same change to methods with promise return types.

**Note:** The changes are disabled for now. I'll add an MC so that we can test this in production in a later diff.

Changelog:
[Android][Fixed] - Make promise NativeModule methods dispatch to NativeModules thread

Reviewed By: PeteTheHeat

Differential Revision: D22489338

fbshipit-source-id: d5b030871f9f7b3f48eb111225516521493cb05e
2020-07-31 12:49:14 -07:00
Ramanpreet NaraandFacebook GitHub Bot ec18e35cd3 Instrument events emitted by TurboModuleManager
Summary:
TurboModuleManager can emit the following events:
- JS Require Beginning
- JS Require Ending
- Module Create (for C++-only TurboModules)

This diff instruments JS Require beginning, and JS Require ending. It also serves as a good stopgap to verify that TurboModule perf logging is set up correctly.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22485529

fbshipit-source-id: a41b88b56627ad2bbcaadac87bf9d530bf07ae81
2020-07-11 09:38:45 -07:00
Ramanpreet NaraandFacebook GitHub Bot 41d948c8d6 Introduce TurboModulePerfLogger Java class
Summary:
We need to instrument the following markers for TurboModules in Java:
- **Java:** moduleDataCreate
- **Java:** moduleCreate

**Problem:** Perf-logging can be on or off in production. This means that we have to guard every perf-logger call, which can be a bit tedious. Therefore, this diff introduces a Java class called `TurboModulePerfLogger`, which:
1. Enables perf-logging by accepting a `NativeModulePerfLogger` `jni::HybridObject` in its `enableLogging` method.
2. Exposes static methods that call into the `NativeModulePerfLogger`'s Java part, when perf-logging is enabled.

We actually have C++ markers as well:
- **C++:** moduleJSRequireBeginning
- **C++:** moduleJSRequireEnding
- **C++:** syncMethodCall
- **C++:** asyncMethodCall
- **C++:** asyncMethodCallExecution

Therefore, `TurboModulePerfLogger.java` also calls its native method `jniEnableCppLogging` to setup C++ TurboModule perf-logging.

TurboModule C++ logging is done via a similar setup (to Java), using `TurboModulePerfLogger.cpp`. The `jniEnableCppLogging` native method calls into `TurboModulePerfLogger::enableLogging` with the C++ part of `NativeModulePerfLogger`. Then, the TurboModules C++ infra uses the static methods on `TurboModulePerfLogger.cpp` to start/end markers.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D22444246

fbshipit-source-id: 66f191056cdcf5d7932ff1916a1de70b82e5f32b
2020-07-11 09:38:45 -07:00
Ramanpreet NaraandFacebook GitHub Bot e5bef7338f Remove TurboModule debug logs
Summary:
These logs are no longer necessary, because data indicates that the TurboModule eager init crash was fixed.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D21852743

fbshipit-source-id: ddeefd6396283ee5e15980a33fb006cb83a81532
2020-06-02 20:11:53 -07:00
Kevin GozaliandFacebook GitHub Bot f24b815fe6 use xplat BUCK attribution
Summary:
Internal code attribution update.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D21603406

fbshipit-source-id: c3da1823e26beb0092d97e66d731618c0433a2f7
2020-05-15 21:55:52 -07:00
Kevin GozaliandFacebook GitHub Bot 164d47f30a label react-native-github targets
Summary:
For internal code attribution.

Changelog: [Internal]

Reviewed By: zlern2k

Differential Revision: D21468924

fbshipit-source-id: 59cd2a52e0ae46bedbf54816820a5f40b684da8b
2020-05-08 00:36:17 -07:00
Ramanpreet NaraandFacebook GitHub Bot 97bc0845ec Guard all debug logs behind an MC
Reviewed By: PeteTheHeat

Differential Revision: D21292280

fbshipit-source-id: 1701a945f126f35a53417b57e3162372d39e75f1
2020-04-29 09:17:17 -07:00
Ramanpreet NaraandFacebook GitHub Bot e171c2b92a Fix cleanup bug
Summary:
When I initially made TurboModuleManager.getModule thread-safe, I unintentionally broken TurboModule cleanup. This diff fixes that mistake.

## Mistake
```
  Override
  public void onCatalystInstanceDestroy() {
    synchronized (mTurboModuleCleanupLock) {
      mTurboModuleCleanupStarted = true;
    }

    final Set<String> turboModuleNames = new HashSet<>(mTurboModuleHolders.keySet());

    for (final String moduleName : turboModuleNames) {
      // Retrieve the TurboModule, possibly waiting for it to finish instantiating.
      final TurboModule turboModule = getModule(moduleName); // ERROR!

      if (turboModule != null) {
        // TODO(T48014458): Rename this to invalidate()
        ((NativeModule) turboModule).onCatalystInstanceDestroy();
      }
    }

```

Before calling `getModule(moduleName)`, I set `mTurboModuleCleanupStarted = true`. This assignment makes all calls to `getModule` return `null`, which means that none of the TurboModules were having their `onCatalystInstanceDestroy()` method invoked.

Changelog:
[Android][Fixed] - TurboModule cleanup

Reviewed By: fkgozali

Differential Revision: D21290582

fbshipit-source-id: 3645b9a4f8c6f41498ebd9d51f9b5775edf2dbd7
2020-04-28 17:47:31 -07:00
Ramanpreet NaraandFacebook GitHub Bot 3d20f49aff Back out "Control concurrent calls into TMMDelegate from TMM"
Summary:
Original commit changeset: d22c4abfe87f

This synchronization is redundant. See D21025965.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D21283284

fbshipit-source-id: dae737fe08bbc8f60913baa6c6a58228611257ef
2020-04-28 14:33:51 -07:00
Ramanpreet NaraandFacebook GitHub Bot 25ed045e36 Switch over to JavaTurboModule::InitParams
Summary:
## Problem
Every time we want to add, remove, or change the data passed to JavaTurboModule's constructor, we have to modify the C++ TurboModule codegen. (The same is true of `ObjCTurboModule`).

**Why was this necessary?**
- `JavaTurboModule` is effectively an abstract class whose constructor is always invoked by code-generated C++ classes. These C++ code-generated class constructors accept an argument list, and manually foward each and every item in that list to `JavaTurboModule::JavaTurboModule`.

## The fix
In this diff, I introduce a struct `JavaTurboModule::InitParams`, to represent a bag of arguments:
```
class JSI_EXPORT JavaTurboModule : public TurboModule {
 public:
  struct InitParams {
    std::string moduleName;
    jni::alias_ref<JTurboModule> instance;
    std::shared_ptr<CallInvoker> jsInvoker;
    std::shared_ptr<CallInvoker> nativeInvoker;
  };
```

All `JavaTurboModules` will be created with an instance of this `InitParams` struct, instead of a list of arguments.  Our code-generated C++ `jsi::HostObject` sublcasses will simply accept `InitParams` in their constructor, and forward it to `JavaTurboModule`'s constructor. This way, the codegen remains oblivious to what arguments JavaTurboModule requires.

## Okay, but why do we need this change now?
In the future, I plan to modify the constructor for `JavaTurboModule` to accept a performance logger, and a `RuntimeExecutor`. Similar modifications are planned for ObjC. For this reason, to avoid these four codemods, and any potential other codemods that occur because we're making modifications to `JavaTurboModule` or `ObjCTurboModule`, I'm launching this codemod, and the codemods in this stack.

## Misc Fix
- Previously, we were generating the TurboModule name from the Spec filename. This is incorrect because that name represents the spec name. Now, the name will be forwarded from TurboModuleManager in the `JavaTurboModule::InitParams` struct.

## Alternative implementations
I initially considered using `ContextContainer`, but decided against it because:
1. There are no type-safety guarantees.
2. I think it's a bit overkill for this scenario. We just need an opaque bag of data, and for our purposes a simple struct does the job fine.

## Commands run

Reviewed By: fkgozali

Differential Revision: D21035208

fbshipit-source-id: 9542cafea192081bc34d337ab3a7a783083eb06c
2020-04-15 23:57:25 -07:00
Ramanpreet NaraandFacebook GitHub Bot bc99a32e4d Control concurrent calls into TMMDelegate from TMM
Summary:
In D20659799, I improved `TurboModuleManager.getModule(moduleName)` thread-safety by ensuring that if two threads race to require the same NativeModule, only one thread creates the NativeModule, while the other one waits until it's created.

## The problem:
What I failed to realize was that when two threads race to require two different NativeModules, we can get concurrent calls into `TurboModuleManagerDelegate.getModule(moduleName)`, and `TurboModuleManagerDelegate.getLegacyCxxModule(moduleName)`, which don't have any thread-safe guarantees.

## The fix
`TurboModuleManagerDelegate` is supposed to be an input to the TurboModule system. So, rather than expecting that all TurboModuleManagerDelegates are thread-safe, which might be a reasonable ask (see T65532092), this diff has `TurboModuleManager` acquire the delegate's lock before calling into it. This ensures that we don't get concurrent access into the delegate, which could be reading from, or writing to, some data structure in these method calls. (This was the case with `ReactPackageTurboModuleManagerDelegate`, which is what Fb4a and Workplace use under the hood).

Changelog:
[Android][Fixed] - Control concurrent calls into TMMDelegate from TurboModuleManager

Reviewed By: mdvacca

Differential Revision: D21025965

fbshipit-source-id: d22c4abfe87f9e534717a06f186dde87d3cd24df
2020-04-14 18:27:16 -07:00
Ramanpreet NaraandFacebook GitHub Bot 36688d35e1 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
2020-04-14 18:27:15 -07:00
Ramanpreet NaraandFacebook GitHub Bot 8901d9518a Add logging to catch null TurboModules
Summary:
We're still seeing NativeModule eager-init crashes in T46487253. So, just to be extra careful, in case this diff doesn't fix the problem, I'm adding logging into `TurboModuleManager.getModule(moduleName)` to see why TurboModules are showing up as `null`.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D21027984

fbshipit-source-id: 74ee62aeac09a4fdb29547e90ef4fa7c07de17a6
2020-04-14 18:27:15 -07:00
Ramanpreet NaraandFacebook GitHub Bot d36fa783a9 Make TurboModule creation thread-safe
Summary:
NativeModules can be created from any number of threads. In the legacy system, `ModuleHolder`, the class responsible for creating NativeModules, has built-in concurrency control to ensure that NativeModule creation is thread-safe. This diff introduces that thread-safety to the TurboModule infra. Basically, after this diff, if `n` threads race to create a TurboModule x, only the first thread will create x. All other threads will wait until x is created.

Changelog:
[Android][Fixed] - Make TurboModule creation thread-safe

Reviewed By: mdvacca

Differential Revision: D20659799

fbshipit-source-id: 2b720fe1ea49e40ae0d6dae50d422f23a6f45520
2020-04-03 18:41:16 -07:00
Kevin GozaliandFacebook GitHub Bot 25f7aea86c Replace fbsource// with // in xplat/js/ files [1]
Summary:
`fbsource//xplat` and `//xplat` are equivalent for FB BUCK targets. Removing extra prefix for consistency.

Changelog: [Internal]

Reviewed By: scottrice

Differential Revision: D20495655

fbshipit-source-id: a57b72f694c533e2e16dffe74eccb8fdec1f55f5
2020-03-25 21:55:47 -07:00
Spencer AhrensandFacebook GitHub Bot 33f20242f6 Hopefully fix so loading crashes
Summary:
Changelog:

[Android][Internal] Fix potential initializer interruption threading crashes.

Reviewed By: mdvacca

Differential Revision: D20615755

fbshipit-source-id: 58b706deeb6df1998caff5bf2ae9ec60114313fe
2020-03-24 11:25:47 -07:00
Héctor RamosandFacebook Github Bot 8ae3174873 Run clang-format and apply patches
Summary:
Run clang-format and add .clang-tidy with `clang-diagnostic-*` to several more directories in order to catch any problems.

Changelog:
[Internal]

Reviewed By: shergin

Differential Revision: D19860169

fbshipit-source-id: 7785aab010c8e6945cc6b5c9b68cb8ee0cdbb7fa
2020-02-27 08:29:30 -08:00