Summary:
changelog: [internal]
Make all feature flags `const` and initialise them as ivars. Also makes the class final and removes virtualised destructor since it is not needed.
Reviewed By: ShikaSD
Differential Revision: D34549013
fbshipit-source-id: 2b326bc5b6c1dd6d89c2fb9c671bda6da669fa76
Summary:
Disables preallocation for cloned nodes, switching to create during commit for those cases instead.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D34555621
fbshipit-source-id: 21fc834cea318ca299aa37a9aab28a2f2a9675b5
Summary:
remove duplicate code in FabricUIManager
The removed line is already called few lines above
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D34553849
fbshipit-source-id: 2e0102a77e7464c9803fb1175385fdafbb88f1fa
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33196
Fixes compilation of Android template and renames the field according to Java guidelines.
Changelog: [Android][Changed] - Rename field with default values for ReactConfig to DEFAULT_CONFIG
Reviewed By: cortinico
Differential Revision: D34523356
fbshipit-source-id: 46ed703e7ca8ab5e05879dd93e8e4d2d816c2d03
Summary:
Introduce DefaultValuesReactNativeConfig constant that represents a ReactNativeConfig that always return default values
I also use this constant in all the apps using fbaric that don't have a custom Mobile Config
changelog: [internal] internal
Reviewed By: RSNara
Differential Revision: D34160478
fbshipit-source-id: 5730054fd4cc87e1fe5dc8247cc237194bf48981
Summary:
With the `MapBuffer`-based props calculated from C++ props, there's no need to keep `rawProps` around for Android views.
This change makes sure that the `rawProps` field is only initialized under the feature flag that is responsible for enabling `MapBuffer` for prop diffing, potentially decreasing memory footprint and speeding up node initialization as JS props don't have to be converted to `folly::dynamic` anymore.
For layout animations, props rely on C++ values, so there's no need to update `rawProps` values either.
Changelog: [Internal][Android] - Do not init `rawProps` when mapbuffer serialization is used for ViewProps.
Reviewed By: mdvacca
Differential Revision: D33793044
fbshipit-source-id: 35873b10d3ca8b152b25344ef2c27aff9641846f
Summary:
Provides an alternative way of diffing props on Android side. Instead of passing dynamic JSON values from JS, the new approach diff C++ props and serializes difference into a `MapBuffer`. Current implementation does this only for `RCTView` component to test performance, correctness and memory impact (as MapBuffers are supposed to be more compact and performant).
This way of passing props allows for additional alignment between platforms, as C++ props are now source of truth for the view state, similar to iOS. At the same time, changing serialization method requires reimplementing `ViewManager` codegen (done manually for now) to account for `MapBuffer`s.
Changelog: [Added][Android] - Added an experimental prop serialization path based on MapBuffer
Reviewed By: mdvacca
Differential Revision: D33735245
fbshipit-source-id: 1515b5fe92f6557ae55abe215ce48277c83974a6
Summary:
Creates a mapbuffer from two ViewProp objects. This MapBuffer is used later instead of bag of props from JS to set the properties with platform ViewManager.
Changelog: [Internal] - Added MapBuffer diffing for ViewProps
Reviewed By: mdvacca
Differential Revision: D33735246
fbshipit-source-id: 10ad46251ea71aa844586624c888f5223fa44e57
Summary:
This is a refactor of the logging of Fabric commit statistics to simplify the way we track performance points.
we'll later refactor and iterate on the API to integrate fabric perf point into developer tools
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D34006700
fbshipit-source-id: 93a01accd90dfacc8b44edd158033b442a843284
Summary:
Was trying out some behaviour when using the MapBuffer experiment and fixed some small issues.
Changelog: [Internal]
Reviewed By: ShikaSD
Differential Revision: D34108859
fbshipit-source-id: 550ca0847419006ec17472cc4b70d38fc8d05396
Summary:
Updates early return in the CREATE command codepath that checks if the view already exists before allocating it. Normally the view state is expected to be created only if the view is preallocated, but empty view state is also created for the flattened nodes when they create an event emitter.
By checking the view existence, we can ensure that view was indeed preallocated before, and skip for optimization purposes.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D34050884
fbshipit-source-id: 489fc1052fec9f71712ea729121ac8ef3e3f3d4e
Summary:
changelog: [internal]
In order to call `RuntimeScheduler::callExpiredTasks`, we need to pass it to `Scheduler` through context container.
Reviewed By: javache
Differential Revision: D34042293
fbshipit-source-id: 62d18507fb107c5be2ac9d003f63735aab6a09ac
Summary:
Venice uses `SurfaceHandler` abstraction which start/stops surfaces independently from `Binding.cpp`, so previous `onSurfaceStart/Stop` callback would not be triggered.
On Android, each surface is used exactly once at the time of writing, so we can use `register/unregister` callbacks to create/clear remembered views for the surface.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D33845685
fbshipit-source-id: 8de4204c7498176fdbe8d44fbc5f2e4079212a1c
Summary:
D33740360 (https://github.com/facebook/react-native/commit/16ed62a850dd81bd9cc7f77ab3e77f42ed64b177) broke Explore VR on React Native. The app would go into a loop on boot and not finish mounting. This is probably a product code issue, but it's not a trivial issue to solve. Unlanding to unblock the RN migration.
Changelog:
[internal] internal
Reviewed By: mdvacca
Differential Revision: D33918026
fbshipit-source-id: cc77c70ece9994d82c91f7ae8783e959629e9cfb
Summary:
Changelog: [Internal]
Remove all the MCs that enable SVCs in Fabric, because we'll only test SVCs in Bridgeless mode to simplify rollout. There were complications with enabling SVCs in Fabric at a previous rollout.
Reviewed By: RSNara
Differential Revision: D33861243
fbshipit-source-id: fdbfedce77f8bd1bab2a807237017787ae8bf7c1
Summary:
changelog: [internal]
Yielding in RuntimeScheduler is shipped. This diff removes the gating around it.
Reviewed By: sshic
Differential Revision: D33740360
fbshipit-source-id: 267372e81e66dda96e451435954a7c3546cc6fbe
Summary:
Adds a check to verify that we are not trying to insert a new view id into a non-existent set.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D33621031
fbshipit-source-id: 8468af69bea250a70d656789ea819c39b55a9de6
Summary:
Adds an experimental replacement for preallocation revision checks on Android mounting layer. Instead of checking revisions of props, we remember tags of preallocated/previously allocated views and use this as a filter to avoid issuing extra create and preallocate commands on copy.
This method is a bit more expensive on memory, but is more precise than relying on revisions.
Changelog: [Internal]
Reviewed By: sammy-SC, mdvacca
Differential Revision: D33038768
fbshipit-source-id: fc98c2a7912e162a3845ca80488635bffaf8cc7f
Summary:
Renaming the `better` utilities to `butter`:
- to prevent claims that this library is superior to others - it really depends on use cases
- to indicate ease of use throughout the codebase, easily spread like butter
Changelog: [C++][Changed] Renaming C++ better util to butter, used by Fabric internals
Reviewed By: JoshuaGross
Differential Revision: D33242764
fbshipit-source-id: 26dc95d9597c61ce8e66708e44ed545e0fc5cff5
Summary:
The previous diff shows how `overflowInset` could improve hit test algorithm performance. This diff adds experiment to it in order to:
1. Understand the perf improvement in production
2. Provide quick way to rollback in production -- the changes are used by FB4A as well as VR
3. The changes will pass more instructions over JNI, which may have impact on perf.
To share the MC param values in both Java and C++ side, the value is hosted by `ReactFeatureFlags` and fetched in `FbReactInstanceHolder`.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D33179310
fbshipit-source-id: 327100d41f7b5a668ff0d2afabcdd1fc16cb5a18
Summary:
This diff adds `overflowInset` values in RN Android. These values are used to give an enlarged boundary of a view group that also contains all its children layout. Here is [the post](https://fb.workplace.com/groups/yogalayout/permalink/2264980363573947/) that discuss more on why this is useful. I steal the pic in that post here as TLDR:
{F687030994}
In the above case, we will get overflowInset for ViewGroup A as something like `top: 0, right: -20, bottom: -20, left: 0`.
This has been added in the [Fabric core](https://fburl.com/code/f8c5tg7b) and [in IOS](https://fburl.com/code/vkh0hpt6). In Android, since we used to ignore all event coordinates outside of a ViewGroup boundary, this is not an issue. However, that caused unregistered touch area problem and got fixed in D30104853 (https://github.com/facebook/react-native/commit/e35a963bfb93bbbdd92f4dd74d14e2ad6df5e14a), which dropped the boundary check and made the hit test algorithm in [TouchTargetHelper.java](https://fburl.com/code/dj8jiz22) worse as we now need to explore all the child node under ReactRootNode.
This perf issue is getting obvious when a view loads too many items, which matches our experience with "Hover getting slow after scrolling", "Hover getting slow after going back from PDP view", and "The saved list view (in Explore) is very fast (because it has very few components)"
To fix this issue, I added the support to `overflowInset` to RN Android by
1. Sending the `overflowInset` values from Binding.cpp in RN Android as a separate mount instruction
2. Update `IntBufferBatchMountItem.java` to read the int buffer sent over JNI, and pass the `overflowInset` values to `SurfaceMountingManager.java`
3. Creating new interface `ReactOverflowViewWithInset.java` and extending the existing `ReactOverflowView.java` usages
4. Adding implementation of getter and setter for `overflowInset` in various views
5. Update `TouchTargetHelper.java` to read the values and check boundaries before exploring ViewGroup's children
Note that in #3 I didn't change `ReactOverflowView.java` interface directly. I am concerned about backward compatibility issues in case this interface is being used in OSS. I suggest we deprecate it as we are not using it anymore in our code.
Changelog:
[Internal][Android]
Reviewed By: JoshuaGross
Differential Revision: D33133977
fbshipit-source-id: 64e3e837fe7ca6e6dbdbc836ab0615182e10f28c
Summary:
Binding.cpp shares many responsibilities, including surface management and mount. This change refactors mount (and mount items) into separate files, while retaining the same functionality and interface.
Changelog:
[Internal][Android] - Refactor mount into FabricMountingManager
Reviewed By: mdvacca
Differential Revision: D32977902
fbshipit-source-id: 07cdf5a19033fccaa81901e5b9a4d44192ec7853
Summary:
This diff adds extra logging to track how long does it take to exeucte different stages of the fabric commit
changelog: [internal] internal
Reviewed By: philIip
Differential Revision: D33183070
fbshipit-source-id: 69e31bef69c9d9ec57dc7e00e4c36e278eb30cb6
Summary:
Scheduler and JavaUIManager mutexes share the lifetime (between install-uninstall), so we can use singular shared lock to ensure that both sections are locked exclusively for install/uninstall and in shared mode for access.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D33130686
fbshipit-source-id: bf283fd5410e386d2635645e17680a9f4cb7f288
Summary:
Fabric uses a cache where it stores the result of the text measurement in C++ (to avoid unnecessary text measurement that are very costly). This cache has a "max size" of 256 and this size is not enough to store all the texts we have in the screen
In my tests, the amount of texts being measured are ~290 and after scrolling many times they increase to 611.
This diff increases the size of the TextMeasure to 1024 for users in the experiment. As a result this improves performance of HoverEvents by +5x times (see test plan)
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D33112788
fbshipit-source-id: e15feecf0f54da62b252892d37a64fb4ead29e22
Summary:
With the updates to touch processing rolled out, we can remove the feature flag and clean up the old code. The old path is now used exclusively by legacy renderer and Fabric uses new `EventEmitter#receiveTouches` method to process touches.
Changelog:
[Changed][Android] - Update touch processing internals
Reviewed By: mdvacca
Differential Revision: D32953664
fbshipit-source-id: 517a4ce6ce9bc15528c2db94d7d11bdff8b78743
Summary:
Applies suggestions from clang-tidy. The automatic linter seems to be broken atm, so I used default config from Android Studio, filtering out meaningless suggestions.
Changelog: [Internal]
Reviewed By: cortinico, sammy-SC
Differential Revision: D32994138
fbshipit-source-id: e8edf53f0cb8b0eda4ff8bb8bf8f5196827efd68
Summary:
The IDE warning suggests that passing folly::dynamic by value will create a copy on each call.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D32978154
fbshipit-source-id: a47a60c332a9d299eb2110d3537dfab0bc2398b6
Summary:
changelog: [internal]
Background executor has been shipped on both platforms for a long time.
I've kept the flag around because I wanted to run tests and compare Concurrent Mode vs Background Executor. The intention was to see if we can get rid of Background Executor to simplify the threading model.
Since then, React team has moved away from Concurrent Mode towards more gradual rollout of concurrent rendering and it no longer makes sense to do this comparison. Right now, we don't have a concern with concurrent rendering and Background Executor. If we ever want to run the an experiment, this gating will need to be added again.
Reviewed By: javache
Differential Revision: D32674798
fbshipit-source-id: a1e51c9c5b8e48efa4cb0f25379d58e7eb80ccd9
Summary:
Preallocation can take 10-20% of time when creating new nodes. (according to systrace measurements). At the moment, we are executing all preallocation calls in JS thread, potentially slowing down the progress there. Moving them to bg thread is a simple micro-optimization that ensures we return the node to JS as soon as possible.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D32677843
fbshipit-source-id: 3df47ae9aa365a8db720d71e2423c87659e9128c
Summary:
Removes extra .so files by merging built-in components into libfabricjni.so
These components shouldn't be referenced in outside modules, so merging them is trivial atm.
Changelog:
[Internal][Android] - Compile native components into static libraries
Reviewed By: cortinico
Differential Revision: D32677572
fbshipit-source-id: fc1a6c5a2832ee49e438c30856562f85677514ea
Summary:
We have `LOCAL_SHARED_LIBRARIES` that are getting longer and are
making reviewing them on Diffs quite hard.
Having all the list of the dependency on a single line is suboptimal
and it makes hard to find duplicated entries.
I've updated the longest `LOCAL_SHARED_LIBRARIES` to be multilines and
I've sorted the entries here.
Changelog:
[Internal] [Changed] - LOCAL_SHARED_LIBRARIES
Reviewed By: ShikaSD
Differential Revision: D32695127
fbshipit-source-id: f5b381c501ddff083ef9f4baaca6c4c8c9523368
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/32640
This Diff shows how an application can provide a custom Fabric Components Registry.
The idea is to extend the CoreComponentsRegistry from rncore to provide the extra
components that are generated from RNTester.
Please note that this Diff can't be shipped as it is and should be rebased on top of:
- D32493605 (https://github.com/facebook/react-native/commit/aa4da248c12e3ba41ecc9f1c547b21c208d9a15f) As the CLANGFORMAT is disabled for RNTester at the moment
- D32045059 (https://github.com/facebook/react-native/commit/d29f3d2a6b84c17ad057f4e9c8dc35b3fe16fb19) and D32128979 as they're effectively adding the sample component and the iOS code.
Changelog:
[Internal][Added] - Adding a Custom Fabric Component to RNTester
Reviewed By: ShikaSD
Differential Revision: D32498360
fbshipit-source-id: 1a737359498dddb571c8a445bec18e5dbcf53e04
Summary:
This is expected with Suspense/CM enabled. We are measuring the perf impact of extra CREATE instructions atm, and will revert the change if it regresses too much.
Changelog:
[Internal]
Reviewed By: sammy-SC
Differential Revision: D32558829
fbshipit-source-id: 2e0c91be3aba4ca632814739aed6b8964e21b5a8
Summary:
Ensures (under a flag) that CREATE instructions will be issued even if the node was supposed to be preallocated before from create or clone. This addresses cases when differ considers nodes to be deleted->created, but we skip the "create" part because of revision check.
Changelog:
[Internal] Disable node revision checks during mount under a flag
Reviewed By: sammy-SC
Differential Revision: D32440831
fbshipit-source-id: 4da8c4961fd7bc43c8f4166798bdfb5d897184e0
Summary:
I'm adding a `EmptyReactNativeConfig` for Java as we had a similar class in C++.
Ideally inside the Playbook we can allow users to use this class rather than having to
create an anonymous inner class.
Changelog:
[Internal] [Added] - Introduce EmptyReactNativeConfig inside Java
Reviewed By: ShikaSD
Differential Revision: D32277916
fbshipit-source-id: f6bbeb0477681d536dfd89930b732609add7c43a
Summary:
The conversion between std::string and Java strings is failing when initializing the `SurfaceHandler`. Instead of manually converting strings, this change makes native init use built-in fbjni helper instead.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D32281900
fbshipit-source-id: 056fce56b40c036d454925c8734bbf2a16f327ff
Summary:
Some of the prerendered surfaces rely on Android context being present to have correct theming (e.g. for platform colors) and measurements of platform components. This change uses context provided to initialize the surface as themed context before view is attached.
This way it is possible to configure theming with `ContextThemeWrapper` the same way as Litho does it for prerendering. The assumption is that any kind of customization done through Android theme will be applied from prerendering entry point as well.
Changelog: [Internal] - Use context from surface for prerendering
Reviewed By: mdvacca
Differential Revision: D31906091
fbshipit-source-id: 344fc96eb2f85ba5b762bee64d1a29443b3fd1d3
