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
Summary:
Simplify addLifecycleEventListener for the flaky test because we just want to test that listener is working.
Changelog:
[Android][Changed] - Add a spare implementation of addLifecycleEventListener for test purpose.
Reviewed By: PeteTheHeat
Differential Revision: D26749256
fbshipit-source-id: 5af216e6bfa37a15eb189aa24a3df35a7a7112de
Summary:
The purpose of this change is to make TTRC markers work similarly for bridge loading and bridgeless loading so we could compare performance between them.
There are mainly four cases involved:
```REACT_BRIDGE_LOADING_START,
REACT_BRIDGE_LOADING_END,
REACT_BRIDGELESS_LOADING_START,
REACT_BRIDGELESS_LOADING_END
```
First 2 are for beginning/ending of bridge loading which includes creating fragment, loading JS bundle, running JS bundle and creating react instance. Last 2 are for similar purpose with bridgeless.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D26514499
fbshipit-source-id: 65d6f3cc7de9e07a7a3a802dd77138e74c23aa5b
Summary:
This diff cleans listeners on the destruction of the ReactContext.
changelog: [inernal] internal
Reviewed By: JoshuaGross
Differential Revision: D26259929
fbshipit-source-id: 1843cabdac2fa3e67dcc890afd923b82472d8f66
Summary:
This diff clears the internal maps of NativeModuleRegistry during turn down of the bridge.
This is necessary for a proper cleanup of these modules.
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D26239303
fbshipit-source-id: 6e98e5db60a4f54d02e99b03339b03c17ecc183d
Summary:
This caused a few apps to crash on launch.
Changelog: [Android][Fixed] - Crashes due to "Make NativeModules immediately initializable"
Reviewed By: olegbl
Differential Revision: D26278594
fbshipit-source-id: 969a3dc49a843366c4ae6ed19a9233d1e6f39b13
Summary:
## Problem:
NativeModules can only be initialized after we mark them initializable on the NativeModules thread. This work is scheduled in ReactInstanceManager.setupReactContext(), after we schedule the execution of the JS bundle on the JavaScript thread in ReactInstanceManager.createReactContext():
https://www.internalfb.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java?commit=9b58f58b8eb12281567b8d24d6d000e868e61a1f&lines=1256-1257%2C1331%2C1334-1335%2C1333-1334
So, if the timings don't work out, the JavaScript thread could start executing the JS bundle before the NativeModule thread makes all NativeModules initializable. In this case, those NativeModule will just appear to be null in C++/JavaScript.
## Fix
This diff makes all NativeModules initializable immediately after their ModuleHolder is created. ModuleHolder.markInitializable() simply initializes initializes modules that were eagerly created.
Changelog: [Android][Fixed] - Make NativeModules immediately initializable
Reviewed By: JoshuaGross
Differential Revision: D26233372
fbshipit-source-id: a9223ff093da5b80781169be88e6ec9516c7a29b
Summary:
Create V2 EventEmitter that surfaceId can be passed into, with a backwards-compat layer, and some debug-only logging to help assist with migration.
Changelog: [Changed][Android] RCTEventEmitter has been deprecated in favor of RCTModernEventEmitter for optimal Fabric support; RCTEventEmitter will continue to work in Fabric, but there are minor perf implications.
Reviewed By: mdvacca
Differential Revision: D26027104
fbshipit-source-id: 784ca092bbc88d19c82f6c42537c34460d96de86
Summary:
This refactors MountingManager into a minimal API that shims into a more fully-featured SurfaceMountingManager. The SurfaceMountingManager keeps track of surface start/stop, surface ID, and surface Context.
This solves a number of issues around (1) race conditions around StopSurface/StartSurface, (2) memory management of Views, (3)
Concrete improvements:
1. Simpler to reason about race conditions around StopSurface/StartSurface.
2. 1:1 relationship between SurfaceId and all Views/tags.
3. When surface is stopped, all descendent Views can be GC'd immediately.
4. Fixed separation of concerns and leaky abstractions: surfaceId/rootTag and Surface Context are now stored and manipulated *only* in SurfaceMountingManager.
5. Simpler StopSurface flow: we simply remove references to all Views, and the Fragment (outside of the scope of this code) removes the RootView. This will trigger GC and we do ~0 work. Previously, we ran a REMOVE and DELETE instruction and kept track of each View in a HashMap. Now we can simply delete the map and move on.
The caveat: NativeAnimated (or other native modules that go through UIManager). APIs like `updateProps` currently uses only the ReactTag and does not store SurfaceId. This is a good argument for moving away from ReactTag, at least in its current incarnation, but: for now this requires that you do a lookup of a ReactTag across N surfaces (worst-case) to determine which Surface a ReactTag is in.
So, to summarize, the "con" of this approach is that now `getSurfaceManagerForViewEnforced` could be slower. It is used in:
* NativeAnimatedModule calls `updateProps` through UIManager
* FabricEventEmitter calls `receiveEvent` on FabricUIManager directly
* On audit, I could find zero native callsites to `sendAccessibilityEvent` through UIManager
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26000781
fbshipit-source-id: 386ae40c4333f8c584e05818c404868dbee6ce73
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
Summary:
Props are transferred from C++ to Java using ReadableNativeMaps. The current implementation of ReadableNativeMaps creates an internal HashMap the first time one of its methods is executed.
During the update of props ReadableNativeMaps are consumed only once and they are accessed through an Iterator. That's why there is no reason to create the internal HashMap, which is inefficient from performance and memory point of view.
This diff creates an experiment that avoids the creation of the internal HashMap during prop updates, additionally it removes a lock that was being used in the creation of the internal HashMap. I expect this change to have a positive impact in TTRC and memory (in particular for ME devices)
This diff reduces the amount of ReadableNativeMaps's internal HashMaps created during initial render of MP Home by ~35%.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D25361169
fbshipit-source-id: 7b6efda11922d56127131494ca39b5cec75f1703
Summary:
These methods were being striped from proguard, which causes release builds to instacrash
Changelog: [Internal] Protect methods from Proguard
Reviewed By: RSNara
Differential Revision: D25314933
fbshipit-source-id: 173160eab953b7c24e02f5e6ef3bf335c1f85526
Summary:
Add three markers for ReactRootView perf logging: onMeasure, attachToReactInstanceManager, and updateLayoutSpecs.
It is suspected that one or all of these have regressed under Fabric in some cases.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24909635
fbshipit-source-id: 6b6c0cc792c4b2d72f2360d6cffc02ef00e8a88b
Summary:
This PR changes ReadableNativeMap.getNullableValue to return null if key not found, instead of throwing exception. This matches method signature and nullable annotation.
## Changelog
[Android] [Changed] - fix ReadableNativeMap.getNullableValue to match signature
Pull Request resolved: https://github.com/facebook/react-native/pull/30121
Test Plan: RNTester app builds and runs as expected, and getNullableValue will return null instead of throwing exception.
Reviewed By: JoshuaGross
Differential Revision: D24164302
Pulled By: fkgozali
fbshipit-source-id: 572c1d4ae5fd493aa0018c2df1dfc7fc91cb4b6b
Summary:
Fix ReadableArray annotations, because these methods throw ArrayIndexOutOfBoundsException instead of null if index is not found.
## Changelog
[Android] [Changed] - fix ReadableArray null annotations. Possibly breaking change for Kotlin apps.
Pull Request resolved: https://github.com/facebook/react-native/pull/30122
Test Plan: RNTester app builds and runs as expected, and show correct type in when used with Kotlin code.
Reviewed By: JoshuaGross
Differential Revision: D24164326
Pulled By: fkgozali
fbshipit-source-id: 0c3f8fa9accbd32cc71c50befe9330e5201643f6
Summary: Adding two new ReactMarkers for start and end of bridgeless initialization. Creating new ones instead of reusing existing ones to make it easier to differentiate data from the bridge vs. bridgeless, and also because our existing markers 1) aren't very easy to understand, and 2) don't map cleanly to the new architecture.
Reviewed By: PeteTheHeat
Differential Revision: D23789156
fbshipit-source-id: 2ed10769e08604e591503a2bc9566aeb1d0563ed
Summary:
New mechanism to soft-crash, or crash, and collect diagnostics in the mounting layer.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D22971260
fbshipit-source-id: 860cde3effa4a187f10f5dd1488dd41ace65e363
Summary:
After D22801173 (https://github.com/facebook/react-native/commit/9e6ba9ddb8608d4e3a4a80d0138600130b766d4c) has landed, the native mechanisms in NativeAnimated to delay queued items from immediate execution are no longer necessary.
Fabric and non-Fabric animations both look smooth after deleting this code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22807906
fbshipit-source-id: 9241fff84376f6aa9a35049cc40dfd6561effaa1
Summary:
As a followup to D22743723 (https://github.com/facebook/react-native/commit/d53fc8a3cd44c7ec7e6eade985daf3d4feb2d736) on the iOS side, I implement a BackgroundExecutor that can be used from C++ to schedule layouts on their own thread, off the JS and UI thread.
This is a potential perf improvement that we will experiment with.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22826795
fbshipit-source-id: 899bd67fe1b86f52910951e9536b59a1414a304c
Summary:
Enable registerSegment in Venice and verify bundle splitting works for Pokes with Venice.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D22666115
fbshipit-source-id: 74ef830b802634b1019d4371873aba599438de37
Summary:
This diff refactors the destruction of the ReactInstanceManager when the app is experiencing low memory.
As part of this refactor, I setup an experiment to understand at what level of memory pressure is convenient to destroy the RN bridge.
The experiment is divided in six levels described in the following table:
https://pxl.cl/1dzx8https://www.internalfb.com/intern/qe2/fb4a_react_native_memory/android_fb4a_instance_unload_pressure_v2/setup
changelog: [internal] Internal
Reviewed By: JoshuaGross
Differential Revision: D22577553
fbshipit-source-id: 37f8f561099a1ba6239795f5907090ced3b5dd18
Summary:
When a native module registers itself as a lifecycle event listener, ReactContext will immediately call `onHostResume` if the activity is already in a resumed state. We rely on this behavior for things like NativeAnimatedModule, which only enqueues the frame callback in onHostResume. However, ReactContext only does this if `hasActiveCatalystInstance()` returns true - which makes sense, because we don't want to notify listeners if the instance has been destroyed. But in bridgeless mode, `hasActiveCatalystInstance` returns false, and we never call `onHostResume` in this case.
This diff fixes an issue where native driver animations don't work the first time you navigate to a screen with bridgeless mode enabled.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22560314
fbshipit-source-id: 1a60cb482896308e21d6e438eb9a7314f580ad04
Summary:
`UIManagerHelper.getUIManager()` relies on the bridge (CatalystInstance) to get the proper UIManager depending on which renderer is being used. Unfortunately, this means it will always return null in bridgeless mode, where the CatalystInstance doesn't exist. This diff replaces the implementation of `BridgelessReactContext.getJSIModule()` to return the FabricUIManager from the ReactHost/Instance.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22480968
fbshipit-source-id: 640e3f22a5b39b315ed2f0397be3cba39e80529a
Summary:
Expose `resolveCustomDirectEventName` from UIManager interface for Bridgeless Mode+NativeAnimatedModule.
We cannot remove the interface from the Non-Fabric UIManagerModule yet, as it would break downstream open-source dependencies, so I just marked it deprecated.
Note that this still doesn't totally fix issues with Bridgeless mode: generally I have to navigate to a Bridgeless surface, exit it, and then navigate back, and only on the 2nd navigation does everything seem to work properly...
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22483508
fbshipit-source-id: 685126e7e51aa5d0fd60ad5d4ecc44e8c6c3029d
Summary:
These flags haven't been used in months. They were useful to uncover some race conditions, but will not be iterated further. The Venice project will obviate the concerns that sparked these experiments in the first place.
These flags have been hardcoded to false for a while.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22319204
fbshipit-source-id: 09415f3bb1ca56e15f357210e966d0483ff384f2
Summary:
Switch between "Fabric" and "Non-Fabric" modes based on which types of native Views are being attached to animations. Don't allow non-Fabric to drive Fabric animations and vice-versa.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21985411
fbshipit-source-id: fb9bef1e38375b384430b4e0275e7b6d62eda7a4
Summary:
This diff exposes receiveEvent on the UIManager class. This is necessary to support backward compatibility between Fabric and classic RN
changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D21979687
fbshipit-source-id: 1ec75896687d55e699f79c520e21f05fac368ee6
Summary:
In the past, in Fabric (Android), we never called stopSurface. Ever! This is bad for memory and can cause other issues, so... let's not do that.
Instead, when the ReactRootView is being torn down, we check the View ID to see if it's a Fabric RootView, and if it is, we call Fabric's stopSurface method.
Fabric stopSurface only has impact on (1) Fabric mount instructions being executed after that point and (2) tells JS to stop running the surface, on the JS thread, asynchronously.
Anecdotally it looks like all the MountItems involved in deleting and removing views from the hierarchy are executed before stopSurface is called.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21965837
fbshipit-source-id: 5169c5a1e339fd9e016ae1234d8389b2bd30be70
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
Summary:
Removing logs related to T62192299 that we don't need anymore.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21773629
fbshipit-source-id: d16c01f87be3ed7512fe90b6e261b4c7efbd3068
Summary:
This is (part of) a rewrite of D15390384.
This implements the lifecycle interface only for Fabric to signal to NativeAnimatedModule when preOperations are about to run / operations are about to be dispatched.
We will likely want to remove this mechanism entirely and rewrite NativeAnimatedModule in C++ for Fabric/Venice, but for now, I'm not sure of a better solution to unblock.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21698193
fbshipit-source-id: a13d445073911fd63d896202a7a1bfbe1167038a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28851
This diff creates a RuntimeExecutor that uses the bridge and exposes it on CatalystInstanceImpl.
Changelog: [Internal]
Reviewed By: mdvacca, RSNara
Differential Revision: D21051949
fbshipit-source-id: b3977fc14fa19089f33e297d29cedba0d067526d
Summary:
This diff refactors the initial render of Fabric in order to set the layout metrics as we start the surface.
This prevents to create an additional fabric commit during initial render. Also this migth help fixing T63495589 (I wasn't able to repro T63495589 again when using this diff)
changelog: [Internal][Android] Internal change to reduce the amount of commits during initial render of Fabric
Reviewed By: JoshuaGross
Differential Revision: D21330072
fbshipit-source-id: 758c49b52ea4c12d5623b7c7d68c7318f4a6cd83
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28779
Creating a JNI wrapper class for RuntimeExecutor so we can pass it to Fabric and TurboModules in Java.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D21049385
fbshipit-source-id: f833004225d9837acf6ffafd3988f89748cf12aa
Summary:
## Description
When the TurboModule and the NativeModule systems are alive at the same time, after RN cleanup, if a TurboModule is required, we return `null` from TurboModuleManager. This causes `CatalystInstanceImpl.getNativeModule` to do a lookup on NativeModule registry, which throws an `AssertionError`, because the TurboModule isn't found. Instead of throwing an `AssertionError` from `CatalystInstanceImpl.getNativeModule`, this diff instead has that method return `null` in this particular case.
## Rationale
This should eliminate the crashes we're seeing in T46487253.
## Future action
In the future, we should guard `CatalystInstanceImpl.getNativeModule` with an `if (mDestroyed) return null;` statement. This'll ensure that if NativeModules are required after React Native has started cleanup, they'll be returned as `null`. Right now, we either return the destroyed NativeModule object, or create/initialize the NativeMoulde and return it, which is wrong.
Changelog:
[Android][Changed] - Make CatalystInstance.getNativeModule nullable
Reviewed By: JoshuaGross
Differential Revision: D21272029
fbshipit-source-id: 099ad9ab9fa2146299df4cf7f86ae7a8e526bb15
