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
Summary:
SurfaceHandlerBinding uses string from JVM memory when initializing, which is released before it is used. If JVM manages to collect this memory before instance init, it can lead to use-after-free.
Changelog:
[Internal] - fix memory corruption in cxx binding of surface handler
Reviewed By: mdvacca
Differential Revision: D31794515
fbshipit-source-id: 3944ebd065f42a516f036096d07c0126a43c912f
Summary:
These dynamic_casts aren't really giving us much (they have never fired once in dev! and don't run in prod anyway). They also prevent us from disabling RTTI. So, let's get rid of them.
Changelog: [Internal]
Reviewed By: philIip
Differential Revision: D31634895
fbshipit-source-id: 4a9b259837127feb324f64fa3e9e23eb1cc481a6
Summary:
Nearly all of these are identical and these compiler_flags are now centralized in rn_defs.bzl. This should have NO CHANGE on build configuration, the flags have just moved for now.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D31631766
fbshipit-source-id: be40ebeb70ae52b7ded07ca08c4a29f10a0ed925
Summary:
This seems like a remnant of an old refactor. This is passed in, we wrap it with a JMessageQueueThread and then never use it again.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D31506280
fbshipit-source-id: aca01439dcddbe2b44ce80342fa8664f827919c9
Summary:
Propagate event category definition to every event that is using `dispatchModernV2` (gated in production), providing opportunity to override categories of some events if needed. No events are meaningfully affected by this change, as coalesced events (e.g. scroll) are always dispatched as continuous and touch events are handled separately.
Changelog:
[Internal] Expose event category in Event class
Reviewed By: cortinico
Differential Revision: D31276249
fbshipit-source-id: f9a756b3a5cf5897e17209f3d0aed6a1c16cbd2e
Summary:
changelog: [internal]
Retaining `EventEmitter` beyond runtime triggers a crash. Let's try to use raw pointer in `EventEmitterWrapper` to see if it fixes some crashes.
Reviewed By: philIip
Differential Revision: D31307332
fbshipit-source-id: cd059b6c56f8dffe985b3ecb62cdafe823ba1462
Summary:
changelog: [internal]
This is a pre-condition to get rid of `shared_ptr` from `EventEmitterWrapper`. Also saves us a few copies of shared_ptr, this is negligible though.
Reviewed By: mdvacca
Differential Revision: D31307048
fbshipit-source-id: b84654bed2359b66faf3995795e135e88fe51cb6
Summary:
changelog: [internal]
This code assured that view preallocation is only triggered if ShadowNode is cloned from revision 0 to revision 1 (first time ShadowNode is cloned).
Node can go from virtual to view forming in subsequent clones, not just the first one. This is more of a case in Concurrent React where the node can be cloned many times before it is first mounted.
Reviewed By: mdvacca
Differential Revision: D31237719
fbshipit-source-id: 13fe6d10fdc815dbdae785d1d9d86d1c8fd36be4
Summary:
Updates event category deduction to match iOS implementation.
The event priority is used by concurrent mode to prioritize certain events, where Cxx part already assigns the correct priority based on the `ContinuousStart` -> `ContinuousEnd` spans. These spans can be deduced from the touch events, which we do in this implementation.
All events that can be "coalesced" (dispatched through `invokeUnique`) are assigned `Continuous` by default in Fabric core, so scroll/slider change events will never be discrete.
Changelog:
[Internal] Add category deduction to Android touch events
Reviewed By: mdvacca
Differential Revision: D31233233
fbshipit-source-id: f5b039aa137f1b4d2e2b15578bfc29ab6903a081
Summary:
For iOS, event category deduction is done from the C++ code, but the touch events are handled on Java layer in Android. This change exposes the category parameter through the `EventEmitterWrapper` called from Java, allowing to define category for events in the future.
Changelog:
[Internal] - Expose event category through JNI
Reviewed By: mdvacca
Differential Revision: D31205587
fbshipit-source-id: f2373ce18464b01ac08eb87df8f421b33d100be2
Summary:
Displays views from the surface mounting manager after we hit a crash during mounting.
This change should help with debugging mounting crashes (e.g. when view is added before removal)
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D30681339
fbshipit-source-id: f83cecaf8e418a2fa5aa0713513c51bb1be77013
Summary:
Ship libjsi as a standalone dynamic library. This prevents problems
with exception handling caused by duplicate typeinfo across multiple
shared libs, and reduces bundle size by removing duplicate copies of
JSI.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D30599215
fbshipit-source-id: abad1398342a5328daa825f3f684e0067cad7a96
Summary:
The impact of this has proven impressive, and safe. Ship in code and remove feature-flag.
Changelog: [Internal]
Reviewed By: philIip
Differential Revision: D30269561
fbshipit-source-id: 9bb72567cfd881928d14d9bee43cf32b390664fb
Summary:
Update FabricUIManager methods for `SurfaceHandler` to start usual rendering or prerendering based on presence of the view instead of using two methods with same logic.
Changelog: [Internal]
Reviewed By: sshic
Differential Revision: D30346502
fbshipit-source-id: 297f2b4a16dc7af7c36379252bd73e6dc953ff59
Summary:
ReactSoftException makes it seem like the class is an Exception, when it's actually a logger. This rename I think is more appropriate.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D30251735
fbshipit-source-id: 550bd543388e698774ec31753200bd3f722d8c17
Summary:
EZ diff that adds a few TODOs to unify component names between JS - Android - iOS - C++
see task: T97384889
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D30139942
fbshipit-source-id: 91f51d04e7e7ecba7f059f94a121be43d820647d
Summary:
This diff extends the FabricUIManager.getInspectorDataForInstance to return the props of the React Native component associated to the view passed as a parameter.
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D30095406
fbshipit-source-id: 50fdba6636a1f5042dbc113e341c3cb2534a1b04
Summary:
Unfortunately, parsing some props requires stateful context - namely, PlatformColor on Android. We explored several different options but they all seemed inferior to the approach of using ContextContainer, and most would require using global state.
By introducing this change everywhere as early as possible, we can avoid later pain. It is likely that some prop, on some platform, will require this mechanism. We'll be ready for it!
Because we can pass a constref of the ContextContainer through to all props and because the context and context data is never retained by prop parsers, perf and memory hit should be ~0.
This diff contains core changes only. Leaf changes to all props structs and conversions files will be in next diff(s).
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D29838789
fbshipit-source-id: f5090e7f02eb6e8fbe0ef4dd201e7d12104a3e3c
Summary:
In T94154173, when calling ```EventEmitterWrapper->invoke()```, hybrid function ```invokeEvent``` is null, even if we checked that ```mHybridData``` is valid before calling ```invokeEvent```.
**Theory:**
```invoke()``` is called from ```mqt_js``` thread, ```desotry()``` is called from ```main``` thread, which cause multi-thread access of```mHybridData```.
So if ```desotry()``` is called after ```isValid()``` check and before calling ```invokeEvent()```, ```invokeEvent``` could be destroyed and is null.
I can reproduce with above theory:
{F633411001}
**Fix:**
Make functions synchronized so ```mHybridData``` can be thread safe.
Changelog:
[Android][Fixed] - Make mHybridData thread safe
Reviewed By: RSNara
Differential Revision: D29792453
fbshipit-source-id: 8b4c754d53ece933be7b2cf99c6cd026b39e24ad
