Summary:
This diff contains the code from the 35 diff stack - D27210587
This diff implement and integrates Mapbuffer into Fabric text measure system
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D27241836
fbshipit-source-id: f40a780df0723f27da440f709a8676cfcca63953
Summary:
Create MC to gate execution of JS Responder in Fabric Android
MC.react_fabric.enable_js_responder_fabric_android is enabled by default in the server
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26905296
fbshipit-source-id: 82504174394d1e10fd017435cccd38952404fda0
Summary:
## Issue:
Sometimes ReactTextView are vertically displayed as one column with bridgeless.
## Root cause:
After debugging, I found out this is caused by a workaround in 2016 to fix a crash caused by mLayout occasionally being non-null and triggers relayout during setText.
https://github.com/facebook/react-native/pull/7011
## Fix
Revert previous hack, if the crash happens again I'll try to fix it.
Changelog:
[Android][Fixed] - Fix text in ReactTextView sometimes being vertically displayed
Reviewed By: mdvacca
Differential Revision: D26581756
fbshipit-source-id: a373d84dc1ab3d787bda7ec82f2d0865a354cf60
Summary:
This bug doesn't reproduce anymore in v301+. MC has been enabled since december
https://www.internalfb.com/intern/logview/details/facebook_android_javascripterrors/419f8892e7b1a02f205810219ddfc299/trends?selected-logview-tab=All%20Traces&drillstate={%22start%22:%22Thu,%2028%20Jan%202021%2000:59:54%20-0800%22,%22end%22:%22Thu,%2011%20Feb%202021%2000:59:54%20-0800%22,%22constraints%22:[{%22col%22:%22mid%22,%22op%22:%22==%22,%22vals%22:[%22419f8892e7b1a02f205810219ddfc299%22]}],%22context%22:%22facebook_android_javascripterrors%22,%22metric%22:%22count%22}
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D26398484
fbshipit-source-id: ca85ca211f1a38aa2691f150956a27c878d243bc
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:
## 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
Summary:
The flag `clipChildRectsIfOverflowIsHidden` has been set to false for a little over a year. Delete it and associated callsites.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25451696
fbshipit-source-id: a6067b2e25124f6bdef336c2ddead719dd44cca9
Summary:
In practice this has been enabled in production for months and is fine. Remove feature-flag and gating.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25451697
fbshipit-source-id: 9e70db3ed4e00de7b8295d9225d43ba09e4e68e9
Summary:
This has been true for 100% of users for months; clean up the old code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25451470
fbshipit-source-id: feae59ce746869b9d84d6aaa69be10e91181f03a
Summary:
This code and this class isn't needed in Fabric, so don't create it during construction. Lazy-load it when it's needed outside of Fabric.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25450724
fbshipit-source-id: d14d8f03c194716f2aba0e499f3282ad2d1c1d29
Summary:
This diff prevents the pre-calculation of ViewManager's constants for users with static view config enabled.
We still load viewManager classes and create viewManger objects for perf reasons
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D25414068
fbshipit-source-id: a91f6113e35b42625c03d13bd67b63e3f9f75098
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:
The root cause of this bug is a race condition between the onMeasure method and setupReactContext.
ReactInstanceManager.attachRootView() method is responsible of the initialization of ReactRootViews and it is invoked by ReactRootView.onMeasure() method in the UIThread
Important initialization steps:
1. Clear the Id of the ReactRootView
2. Add the ReactRootView to the mAttachedReactRoots
3. Call StartSurface (if the bridge has been initialized)
Sometimes, when this method is invoked for the first time, the bridge is not initialized, in those cases we delay the start of the surface.
Once the bridge is initialized, StartSurface is called by the setupReactContext() running in the NativeModuleThread.
Since onMeasure can be called multiple times, it is possible that we call "StartSurface" twice for the same ReactRootView, causing the bug reported on T78832286.
This diff adds an extra check to prevent calling "StartSurface" twice. The fix is done using an AtomicInteger comparison and it is gated by the flag "enableStartSurfaceRaceConditionFix". Once we verify this works fine in production we will clean up the code, remove the flags and maybe revisit the API of ReactRoot.
changelog: [Android] Fix race-condition on the initialization of ReactRootViews
Reviewed By: JoshuaGross
Differential Revision: D25255877
fbshipit-source-id: ca8fb00f50e86891fb4c5a06240177cc1a0186d9
Summary:
This feature flag (enableStopSurfaceOnRootViewUnmount) was used to gate usage of the "stopSurface" API, which is now fully supported, and has been used
in the FB app for several months. This is safe to ship in code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25275192
fbshipit-source-id: fa22bfd00aa023297bc19c83c138f133e9ff1645
Summary:
This was added to prevent mutating the UI during draw or measure, and appears to have been effective. Keep the comments, ship the feature, remove flags.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25258409
fbshipit-source-id: 36ad8a03d1eb82bc9dcd769372c03f1ebe8b8da8
Summary:
Currently, queuing any sort of MountItem or getting the list of MountItems requires synchronization. Since these can be queued up at any point from the JS thread, there will, in theory, be constant contention between JS and UI thread.
To see if this is true, I'm creating an experiment instead of just switching over to concurrent structures. After seeing results, we can hopefully make a decision and delete the non-concurrent stuff or improve on it somehow.
The original was unlanded in D24973616 (https://github.com/facebook/react-native/commit/26787e2260412d9d2fe831e68a8616505d3cab36) due to a typo, which has been fixed now. The typo was that in Blocking Mode, we queued up all PreMountItems to the concurrent PreMountItems queue.
Changelog: [Internal]
Reviewed By: ShikaSD
Differential Revision: D24974851
fbshipit-source-id: d081c081bbd0de445bb92408f0ec822056b905a5
Summary:
Currently, queuing any sort of MountItem or getting the list of MountItems requires synchronization. Since these can be queued up at any point from the JS thread, there will, in theory, be constant contention between JS and UI thread.
To see if this is true, I'm creating an experiment instead of just switching over to concurrent structures. After seeing results, we can hopefully make a decision and delete the non-concurrent stuff or improve on it somehow.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D24942110
fbshipit-source-id: fcbdeda51f91f4bd430a20d7484854fb1e94a832
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
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
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
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
Summary:
For over a month I've been searching for a crash that occurs during Android's native `dispatchDraw` method on View. The stack traces didn't show anything useful - everything in the stack trace was native Android code,
not React Native code.
This also seems to only repro on certain vendors, and only on a very few React Native screens - I'm still not sure why either of those are the case, but from my repro, a *very* specific set of interactions needs to happen
to trigger this crash. See comments inline and an example stack trace.
Luckily, the fix is trivial. Since this code is used for animations, accessibility, and a number of other important interactions, I'm gating this change for now.
In general we must be careful to only mutate the View hierarchy only when we /know/ for certain it is safe to do so. Indirectly mutating the View hierarchy during measure, onMeasure, layout, dispatchDraw, etc, can all be
very dangerous. This is one of the last "escape hatches" that can cause view hierarchy modifications unexpectedly, so I think it's a very good idea to "secure" this further, and only update props synchronously here - and
ensure that other MountItems like `Delete` are definitely /not/ executed here.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24639793
fbshipit-source-id: b6219ce882e8d2204b4d10bf99f6a1120a33cb5a
Summary:
Fabric should be inserting Views into the hierarchy in the correct order based on z-index already, so there should be no reason to enable this mechanism.
At best it's a perf pessimisation and at worst it could be causing consistency issues or crashing (TBD). Most likely this is a noop.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D24512203
fbshipit-source-id: b9336240ef8506742bcbd8d08fc8b830f82cdfe2
Summary:
Remove `enableFabricStartSurfaceWithLayoutMetrics` and treat as `true` always from now on.
Changelog: [Internal]
Differential Revision: D23633198
fbshipit-source-id: 5b7455b87e578ffa97d80746fa901cd2b50d3ea9
Summary:
This diff optimizes the caching of Spannable objects managed by the TextLayoutManager class.
Previously, these objects were cached using unsing a String representation of the RedableMap (creating this string adds a non trivial cost), this diff improves the caching performance relying on the equals / hashcode methods of the ReadableNativeMap class
I created a MC just to have a killswitch
Motivation: I was analysing another bug and I found this non performant code
changelog: [internal] internal
Reviewed By: shergin
Differential Revision: D23429365
fbshipit-source-id: 59e5ad0b1b95da992ac393aecfe029da68a8df97
Summary:
With BackgroundExecutor+State Reconciliation, it is pretty common to have races where View layer UpdateState calls are thrown away.
Theoretically this is /always/ possible since C++ doesn't retry failed UpdateStates, we just try once and then bail. This is part 1 of improving our mechanisms for ensuring that State stays up-to-date from the View layer.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22933927
fbshipit-source-id: 77b3b87402f772e3f149be0f9200e673bbed7b57
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:
According to our experiments it's not better than "stop surface on unmount" in any way, and might regress some metrics. Unclear why, but if it's not necessary and doesn't seem to help, it doesn't make sense to continue this experiment.
We still have a mechanism on the C++ side to stop outstanding surfaces on teardown that does the same thing.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22318864
fbshipit-source-id: 7e678c63e4884382e57d996a7f4c4b7b24c8193a
Summary:
Early ViewCommand dispatch: ship the experiment everywhere on Android.
Since ViewCommands are totally divorced from the commit cycle currently, and since they are inherently unsafe, we can create a separate queue for them
and retry them if they fail with a specific category of exceptions.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22173050
fbshipit-source-id: 494b7c6b5dfd2aec8ba77ae35d0d58d4d727b9b4
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:
EZ diff to expose a new ReactFlag that will be used to configure the initialization of Fabric
changelog: [Internal] Internal change in fabric
Reviewed By: JoshuaGross
Differential Revision: D21606972
fbshipit-source-id: 53d6bac673b95f0fae93262ff52b815d76bb59ab
Summary:
This diff exposes a new react feature flag to eager initialize fabric
changelog: [Internal] Internal change in Fabric
Reviewed By: JoshuaGross
Differential Revision: D21574814
fbshipit-source-id: c74fb316963fe92e43ce0ca6262cb73a6a4acb7f
Summary:
This diff adds a temporary Feature Flag to control a fix in TextInlineView (see previous diffs of the stack)
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D20812920
fbshipit-source-id: 90fece9b29ba173546d96e4d9baf1ccabb3031b2
Summary:
Hard-coded to false everywhere, and write-only. We never read from this.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20788252
fbshipit-source-id: ae117ebc51db7045947b9713602527ff4220833e
Summary:
Remove unused feature flag. This is not used within Facebook and I'm not aware of usage outside of FB.
Changelog: [Removed] Removing Android feature flag: lazilyLoadViewManagers
Reviewed By: mdvacca
Differential Revision: D20788210
fbshipit-source-id: 435316e3de7830d7cb7f14537351883e4fc6eeaa
David Vacca
Lulu Wu
Andrei Shikov
Ramanpreet Nara
Joshua Gross
Kevin Gozali
Alessandro Sisto
generatedunixname89002005287564