Summary:
After the change, YogaLayoutableShadowNode will preserve dirty flag (being false) in cases where:
* a node was cloned with same children;
* changes in props don't affect layout.
Motivation:
In Fabric we always were aggressive about dirting yoga nodes: when we clone the node, we always dirty underlying Yoga node. I think that was the case because we don't deeply understand how the system works and we always had more severe problems to fix. Now, we faced an issue that forces us to think about that problem carefully (more about that later).
(I don't expect that will improve TTI performance, but that's possible if we do a lot of changes in the hierarchy during initial load process.)
I see two main use cases where we have to be smart about dirtied yoga nodes:
Case 1. Native local commits which do *not* modify yoga styles.E.g. in a case where ScrollView updates the internal state (which has offset info) really really frequently. (It will be implemented later.) That `contentOffset` info does not or might not affect layout, so we should not pay for this.
Case 2. Legit React commits which do *not* modify yoga styles. E.g. in a case where only the background color of some view changes, we don't need to relayout anything. Unfortunately, we do this in Fabric because of two major reasons:
* We don't make a difference between any changes in Props.
* React constructs new children of cloned node iteratively, calling `appendChild`. That makes implementing optimization really challenging because we don't know when the process ends. And when it ends we have very few pieces of information about how the state looked before.
This diff stack will handle the problems of the first kind. The main motivation is: we want to have state updates to be as lean as possible.
Reviewed By: JoshuaGross
Differential Revision: D14472752
fbshipit-source-id: 68374f60cb07de9ab65bf1f6d94c828985359fa5
Summary:
This is the couple of hacks I used after I finished #23802 in order to get fabric working on RNTester. This is inspired from prior work by kmagiera.
The goal of this PR is to show others what I’m struggling with, and to eventually merge it sans hacks.
- Yarn Install
- Uncomment the commented out pods in RNTester's pod file
- Open RNTesterPods workspace
- Run App
- this is only for pods, the non-pod RNTester will no longer work until updated with fabric too.
- `SurfaceHostingView` & `SurfaceHostingProxyRootView` both try to start the surface immediately, this leads to a race condition due to the javascript not having loaded yet, the hack here is:
1. Swizzle the `start` method on `RCTFabricSurface` to no-op when called.
2. Add observer for `RCTJavaScriptDidLoadNotification`
3. Call private method `_startAllSurfaces` on `_surfacePresenter` in AppDelegate when we receive `RCTJavaScriptDidLoadNotification`.
[General] [Added] - Use Fabric in RNTester
Pull Request resolved: https://github.com/facebook/react-native/pull/23803
Reviewed By: shergin, mdvacca
Differential Revision: D14450726
Pulled By: fkgozali
fbshipit-source-id: 8ae2d48634fecb60db539aaf0a2c89ba1f572c27
Summary: Several things need to ironed out: 1) LocalState in Fabric C++, 2) setting dimensions of BottomSheet component to 0,0 for parent.
Reviewed By: shergin
Differential Revision: D14426167
fbshipit-source-id: 45a90a7971c87672872108a9e360926b4a6095f0
Summary:
Accidentally I noticed that the signature of Folly's hash_combine is different from boost's one.
The Folly's one is:
`size_t hash_combine(const T& t, const Ts&... ts)`, so the first argument is immutable and the method returns the result normally.
It means that all hashes that we compute in Fabric using `hash_combine` were `0`.
So I fixed it.
I have no idea why this difference exists, but some modern papers suggest that folly's variant has good chances to be standardized.
E.g.: http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0814r0.pdf
Technically, it should improve performance, but I doubt that it can be more than 1-2 ms per screen TTI.
Reviewed By: JoshuaGross
Differential Revision: D14430380
fbshipit-source-id: 97da999ee5780b940bb789bc3eb5bf9f89c194ca
Summary:
While the proper typographical terminology is [Strikethrough](https://en.wikipedia.org/wiki/Strikethrough), we should still support `line-through` as it goes through a deprecation phase.
[iOS] [Added] - line-through support for fabric text components
Pull Request resolved: https://github.com/facebook/react-native/pull/23911
Differential Revision: D14460610
Pulled By: shergin
fbshipit-source-id: 0dae41b765f21d166ea3618c463ebe1330607b30
Summary:
Moves rn_codgen to rn_library behind a `codegen` flag to limit the number of rules we generate
WIth this, modules with native components can enable the react-native-codegen schema parsing by just updating their buck rule to:
```
rn_library(
name = 'lib',
codegen = True,
// ...
)
```
Reviewed By: TheSavior
Differential Revision: D14401263
fbshipit-source-id: 1675bc28389db64da10153c988bb4eb00d715056
Summary: This diff is a part of an effort to resolve build errors that appear when compiling with the latest version of clang.
Reviewed By: shergin
Differential Revision: D14369797
fbshipit-source-id: e4c6f53e293f336efad18597f9d2705c025e1051
Summary: The hope is that it will remove many unnececery allocations improving overal perfromance.
Reviewed By: mdvacca
Differential Revision: D14249198
fbshipit-source-id: f0442b3919ccead0582a3190dea0e33d517d85f6
Summary:
SharedShadowNodeList is one of the core collections in Fabric. Every ShadowNode has one, it's used intensivly during diffing and so on.
Usually, nodes have a very few children, so using `small_vector` instead of regular `vector` should save us a lot of memory allocations.
Reviewed By: JoshuaGross
Differential Revision: D14249201
fbshipit-source-id: 53297caa027a74099d0a1ed4a3cce78f8feb651b
Summary:
Nowadays, every ShadowNode has a reference to a ComponentDescriptor, so now there is no need to do registry lookup for that.
That should save us 0.5-1.0 ms in my non-scientific measurements.
Reviewed By: mdvacca
Differential Revision: D14348369
fbshipit-source-id: 57f6a6f2f8bf0b7e58d89a414fec20b2db8876f7
Summary:
Clowntown.
This fixes a recent regression happened becasuse wrong order of parameters in ShadowViewMutation constructor.
Reviewed By: JoshuaGross
Differential Revision: D14329164
fbshipit-source-id: 5a0dc04b889fb3357050b951d56c3e436dbeefb1
Summary: Use the codegen for the Slider component with the new `inferfaceOnly` option
Reviewed By: TheSavior
Differential Revision: D14295981
fbshipit-source-id: 0482572892fbcffada43c7c6fbf17e70546300b8
Summary: These were moved to the codgen
Reviewed By: TheSavior
Differential Revision: D14307470
fbshipit-source-id: 49030c245f11bd595bb97e1b2f34a7c454be5c56
Summary:
This pull request removes the designated initializer in `LayoutMetrics.h`. This will help improve the portability of this file.
[General] [Changed] - Fabric: Remove designated initializer in LayoutMetrics.h
Pull Request resolved: https://github.com/facebook/react-native/pull/23758
Differential Revision: D14320526
Pulled By: shergin
fbshipit-source-id: 076ad389d0985d5213b521a2ffaca4ca7ae31599
Summary:
This pull request removes the use of the GCC extension null coalescing operators (`?:`) and replaces them with ternary operators. This improves the portability of `ShadowNode.cpp` (and enables MSVC to build it)
[General] [Fixed] - Fabric: Removed null coalescing operators in `ShadowNode`
Pull Request resolved: https://github.com/facebook/react-native/pull/23438
Differential Revision: D14304958
Pulled By: shergin
fbshipit-source-id: 7d8e6778a72dabf09b1d99bc091a7578598b79c3
Summary:
This pull request removes the designated initializers in `RootShadowNode.cpp`. This will help improve the portability of this file.
[General] [Fixed] - Removed designated initializers in `RootShadowNode`
Pull Request resolved: https://github.com/facebook/react-native/pull/23715
Differential Revision: D14305167
Pulled By: shergin
fbshipit-source-id: e394580f103fdb59cf078828b5d2ee6df6cc534d
Summary:
This pull request removes the designated initializers under the `fabric/uimanager/` folder. This will help improve the portability of these files.
[General] [Fixed] - Removed designated initializers under the `fabric/uimanager/` folder
Pull Request resolved: https://github.com/facebook/react-native/pull/23717
Differential Revision: D14305203
Pulled By: shergin
fbshipit-source-id: 370911682e22cbb20c4bfd7382ed4da0ce1a598d
Summary:
This pull request removes the designated initializers in `ShadowNodeTest.cpp`. This will help improve the portability of this file.
[General] [Fixed] - Removed designated initializers in `ShadowNodeTest`
Pull Request resolved: https://github.com/facebook/react-native/pull/23714
Differential Revision: D14305162
Pulled By: shergin
fbshipit-source-id: 81803652ece734a9b223d7a8b462a8b62cde7d1d
Summary:
This pull request removes the designated initializers in `ComponentDescriptorTest.cpp`. This will help improve the portability of this file.
[General] [Fixed] - Removed designated initializers in `ComponentDescriptorTest`
Pull Request resolved: https://github.com/facebook/react-native/pull/23713
Differential Revision: D14305123
Pulled By: shergin
fbshipit-source-id: 64cfc76dd7eaf74b4b4395bac18a9fc370a3b322
Summary:
I am not sure why it compiled before, it clearly should not, IMO. The `const` types (and references!) are not allowed inside `std::vector` because they are not assignable.
Some experiments that I did caused compilation errors here, so I am changing that to be actually correct.
Reviewed By: JoshuaGross
Differential Revision: D14249199
fbshipit-source-id: 07a22ef13f5de9dfc7ab307493419e6006994bc2
Summary:
This pull request removes the designated initializers in `react/mounting/**` and `react/events/**` to improve portability.
A destructor was also defined for `ShadowView` to fix this error:
```
ShadowViewMutation.cpp:14: error: undefined reference to 'facebook::react::ShadowView::~ShadowView()'
ShadowViewMutation.cpp:24: error: undefined reference to 'facebook::react::ShadowView::~ShadowView()'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
```
[General] [Changed] - Fabric: Remove designated initializers in Events and Mounting
Pull Request resolved: https://github.com/facebook/react-native/pull/23441
Differential Revision: D14298890
Pulled By: shergin
fbshipit-source-id: f5d8fc6e1f5968b94e8bb3ca0c3f0e81cf892f83
Summary: This diff removes the "isLayoutable" parameter from SchedulerDelegate.schedulerDidRequestPreliminaryViewAllocation. This now can be infered from the shadowView parameter
Reviewed By: shergin
Differential Revision: D14296481
fbshipit-source-id: b200504f9c2bef41f0a70257f1f5a274fbe97cbb
Summary: It can save us a couple of ms, hopefully.
Reviewed By: mdvacca
Differential Revision: D14249196
fbshipit-source-id: b5911bcd8b49be66de7b9d2da660df38ef7cc8cd
Summary: That should reduce the size of `ShadowNode`, make an instantiation of shadowNode a bit faster, and make `clone` operation a bit faster because simple dynamic dispatch is faster than std::function invocation. It also should help a bit with code size.
Reviewed By: mdvacca
Differential Revision: D14249200
fbshipit-source-id: c1332b139d27becebf15cd894475507b5fd0eb9f
Summary: We need this change to migrate away `cloneFunction` (to a pointer to ComponentDescriptor) inside ShadowNode.
Reviewed By: mdvacca
Differential Revision: D14249197
fbshipit-source-id: 773edcf40e17989886e2c5d3955823a0dbf3857a
Summary: Surprisingly, we have some significant amount of text measuring requests where the string is empty. So, there is no need to go to platform specific layer to find that the size of those strings is zero.
Reviewed By: mdvacca
Differential Revision: D14297315
fbshipit-source-id: bf84cf27d5c0893262e8b27da8ff42fc77bcd6c5
Summary: `ParagraphMeasurementCache` was replaced with templated version of itself.
Reviewed By: mdvacca
Differential Revision: D14296515
fbshipit-source-id: 29e370f07baf14b25430f85a06f603907aed5563
Summary:
Creation NSAttributedString from attributedString is not so cheap process, so with this simple cache we hopefully can save a couple milliseconds.
The same string is used at least two times: first time for measuring and second for drawing.
Reviewed By: mdvacca
Differential Revision: D14296514
fbshipit-source-id: 6313aa2c6e9f63d873868131750f61c02d64d2de
Summary:
Update Props during pre-allocation avoiding re-updating the same props during mounting
MobileLab test showed an improvement of:
MARKETPLACE_YOU_TTI_SUCCESS: -3.34% = 58ms
Reviewed By: shergin
Differential Revision: D14252170
fbshipit-source-id: 1f4e9ad5dcecbc06651fa065135ffeed4892d984
Summary: This diff introduces the concept of "updateProps" as part of InsertMutation and it changes the diffing algorithm to populate this field.
Reviewed By: shergin
Differential Revision: D14289608
fbshipit-source-id: 642f00d03d294a12ea7fa7482c72e701b756f3d4
Summary:
This diff changes the pre-allocation of Images update the props on the ImageViews during the creation of ShadowNodes instead of during rendering.
The purpose of this change is to optimize TTI.
This was originally landed in D14214844 and then backed out in D14247897
Reviewed By: shergin
Differential Revision: D14286232
fbshipit-source-id: 8437f57f9473eb22ef98d80531b4020ee5fbb9ae
Summary:
Props now have a special value that represents generation number of `Props` object, which
increases when the object was constructed with some source `Props` object.
Reviewed By: mdvacca
Differential Revision: D14293939
fbshipit-source-id: 4782bf33ccf37623d4079c09cc4d0268bb6c2690
Summary: Add assorted missing includes in `xplat` that would be exposed by future changes.
Reviewed By: ispeters, nlutsenko
Differential Revision: D14213660
fbshipit-source-id: 329f133784015fe20ee99feaec8ef05e117fe3a6
Summary:
This is a back-out of D14214844, we noticed that this regressed TTI for Marketplace You screen running in Fabric
Original commit changeset: b81005f2bf49
Reviewed By: JoshuaGross
Differential Revision: D14247897
fbshipit-source-id: de0cea92b437b2fbcd075f0d6a0066156800e3f0
Summary: Small perf wins for both iOS and Android.
Reviewed By: PeteTheHeat
Differential Revision: D14237532
fbshipit-source-id: 2be114d36adfa6e8540cb8cbca5412782791d8ce
Summary:
In React Native there are several use cases where React State is not the only input that affects the component tree. E.g., in case of a <Modal> component, the screen size directly affects the layout of components inside modal; in the case of uncontrolled <TextInput> component, the text inside the input affects the layout of the surrounding components. `State` is a special (legit!) workaround for all those similar cases. Native part of React Native maintains a special shared object between all nodes of the same family and use that during cloning and commits.
See coming commits to know how to use that.
In the near future State will fully replace LocalData concept but for simplicity they both exist for now.
Reviewed By: mdvacca
Differential Revision: D14217184
fbshipit-source-id: 6e018c5b68208d662462013bce0f4e2733d2f673
Summary:
A couple of small changes:
1. `onEnqueue` was decoupled from `enqueueEvent` for easier and unified overiding in subclasses (other methods will call `onEnqueue` soon);
2. `flushEvents` was decoupled from `onBeat` (we will put more stuff into `onBeat` soon).
Reviewed By: mdvacca
Differential Revision: D14205769
fbshipit-source-id: 574c2b2caaa6432bc7782b2f3bc147fa1fb82bd3
Summary: `LayoutConstraints::clamp` clamps the provided `Size` between the `minimumSize` and `maximumSize` bounds of this `LayoutConstraints`.
Reviewed By: mdvacca
Differential Revision: D14205770
fbshipit-source-id: 4799608cead393451d334e47dd6906255699a1e8
Summary: That's bummer that we have to do it, but it's actually reasonable. Files in `core` and `events` depend on each other creating circular dependencies and other similar hard problem.
Reviewed By: mdvacca
Differential Revision: D14195022
fbshipit-source-id: 96a44ae28631cc9ccd7d7de72a94526f9e0dd12a
Summary:
This diff changes the pre-allocation of Images update the props on the ImageViews during the creation of ShadowNodes instead of during rendering.
The purpose of this change is to optimize TTI.
Reviewed By: shergin
Differential Revision: D14214844
fbshipit-source-id: b81005f2bf494f62f421dc24846e1561e13b9a87
Summary:
Not super clean, but not terrible.
Unfortunately this still relies on the old Paper UIManager calling delegate methods to flush the operations queues. This will work for Marketplace You since Paper will be active, but we need to fix this, along with Animated Events which don't work at all yet.
Random aside: it seems like taps are less responsive in fabric vs. paper, at least on iOS. There is a sporadic delay between the touches event coming in nativly to the JS callback invoking the native module function to start the animation - this will need some debugging.
Reviewed By: shergin
Differential Revision: D14143331
fbshipit-source-id: 63a17eaafa1217d77a532a2716d9f886a96fae59
Summary: It's handy to have all in one file but it also make it hard/impossible to fight with circular dependencies, so I had to slit it.
Reviewed By: mdvacca
Differential Revision: D14188546
fbshipit-source-id: 20809f1ea227c6f8f32922ed5760226248b718e3
Summary:
The previous implementation of the method cloned the root node twice (one time at the very end of the method and one time at the end of loop body).
The new one does it once and a bit more readable.
Reviewed By: mdvacca
Differential Revision: D14187969
fbshipit-source-id: 9859deadd4b041ac115c37108188aab70200c75d
Summary:
RootShadowNode does not have ComponentDescriptor counterpart (because it cannot be created via UIManager) and we used to always clone it manually.
However, apparently, some algorithms (e.g. clone algorithm inside RootShadowNode, which is used by setNativeProps) require valid clone function associated with the node.
So, we implement it.
Reviewed By: JoshuaGross
Differential Revision: D14187762
fbshipit-source-id: a9b6b332a18583217ff0e4f9c15aea0ffb113ba2
Summary:
The ConcreteViewShadowNode was changed to be independend of actual amount of template arguments of ConcreteShadowNode.
We will use it soon.
Reviewed By: JoshuaGross
Differential Revision: D14187761
fbshipit-source-id: b4c8051e2ae3803932713b0c255492466e80d3bd
Summary:
* "rotate" is often used as shorthand for "rotateZ"
** Paper handles this here: diffusion/FBS/browse/master/xplat/js/react-native-github/React/Views/RCTConvert%2BTransform.m$89
* Sometimes react sends a string with units, e.g. "45deg", so we need to convert that to a Float.
** Paper handles this here: diffusion/FBS/browse/master/xplat/js/react-native-github/React/Views/RCTConvert%2BTransform.m$14-27
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D14154173
fbshipit-source-id: 53d7405f26c78bb470d46879309c9697d9985c1c
Summary: Currently xcode fails profile builds since these variables are unused when asserts are disabled.
Reviewed By: PeteTheHeat
Differential Revision: D14160565
fbshipit-source-id: 997352dd148d23c28fa92d4171071c1abbb742f5
Summary: Trivial. If it compiles, it works.
Reviewed By: JoshuaGross
Differential Revision: D14146579
fbshipit-source-id: 6f6895d1634709a5bde012850c5df756171320ab
Summary:
@public
Encapsulates node cloning within `YGConfig`.
This is necessary for allowing for context-aware cloning functions, which will ultimately allow for removal of weak global JNI references.
Reviewed By: shergin
Differential Revision: D14132608
fbshipit-source-id: 0dec114c8e172b1e34a4b7fd146c43f13c151ade
Summary: Use the new copyright header format used elsewhere in the React Native repository.
Reviewed By: shergin
Differential Revision: D14091706
fbshipit-source-id: b27b8e6bcdf2f3d9402886dbc6b68c305150b7d5
Summary:
This changes fix small perf problem in RCTImageManager (and layout!) saving (in my completly non-scientific tests) up to 2 ms.
Maybe that's not much, but it fires during layout (and ShadowNodeTree reconsiliation processes), so it has to be very performant.
Reviewed By: JoshuaGross
Differential Revision: D14072077
fbshipit-source-id: 0baa54584dc3ae6a5f43be13fd152b84801c8539
Summary:
Sometimes, when we deal with ImageRequest and ImageResponseObserverCoordinator we subscribe for status (or access the coordinator) without owning an ImageRequest. In those cases, we have to retain the coordinator explicitly.
For those cases, ImageRequest now exposes `ImageResponseObserverCoordinator` as a `std::shared_ptr`.
Eg, concretely in the code, `completionBlock` and `progressBlock` copied a raw pointer to the observer inside which can lead to a crash when ImageRequest is being deallocated before we received an image data.
Reviewed By: JoshuaGross
Differential Revision: D14072079
fbshipit-source-id: e10120bc05bf685e288f7b3d69092714dcd91d43
Summary:
This diff refactors JNI methods used by the Binding.cpp class in order to use ReadableMap instead of ReadableNativeMap
This will be helpful to provide a different implementation of ReadableMap from C++
Reviewed By: shergin
Differential Revision: D14077762
fbshipit-source-id: 595b0c2d3a2d6070112257b65c1141a8af36f0e1
Summary:
This pull request implements some suggestions made in #23414, namely:
* Rename `ShadowNodeFragment::nullSharedProps()` (and family) to `ShadowNodeFragment::propsPlaceholder()`;
* Introduce similar functions for the rest of members (tag and rootTag);
Later pull requests to remove designated initializers from Fabric (for MSVC support) will use these
I'd like to do the rename of `rootTag` to `surfaceId` as well, but would make sense to do in a separate diff as that would result in a great deal of changes.
[General] [Changed] - Changed naming scheme of `ShadowNodeFragment` placeholders, and added placeholders for remaining members
Pull Request resolved: https://github.com/facebook/react-native/pull/23437
Reviewed By: mdvacca
Differential Revision: D14072007
Pulled By: shergin
fbshipit-source-id: d5bb10a82c16d45955f417a49b503f215c4bc521
Summary:
This pull request removes a constructor in `DebugStringConvertibleItem.h` that was causing this issue in MSVC:
```
DebugStringConvertibleItem.cpp(20): error C2600: 'facebook::react::DebugStringConvertibleItem::DebugStringConvertibleItem': cannot define a compiler-generated special member function (must be declared in the class first)
```
It was likely conflicting with the constructor that has default values for all of its arguments.
[General] [Fixed] - Fixed `DebugStringConvertibleItem` compilation on MSVC
Pull Request resolved: https://github.com/facebook/react-native/pull/23436
Reviewed By: shergin
Differential Revision: D14067760
Pulled By: mdvacca
fbshipit-source-id: 303cf9b3559932c3d06514a1f3a8739d0f6f9dc2
Summary: Now in BUCK file only, not in code.
Reviewed By: JoshuaGross
Differential Revision: D14019271
fbshipit-source-id: e1396be7156a374a1379a147ddecb83b51686121
Summary:
It's better to comment `DWITH_FBSYSTRACE` out in BUCK files instead of removing them from the code.
I'll publish the BUCK changes as separate diff for simpler backout in the future.
Reviewed By: mdvacca
Differential Revision: D14019272
fbshipit-source-id: 8b322b5c115efe33c15929e008b97a05220813df
Summary:
Trivial.
If you have troubles with rebasing on top of this revision, run this on your diff:
$ find */*.h */*.mm */*.cpp */*.m -exec clang-format -style=file -i {} \;
Reviewed By: JoshuaGross
Differential Revision: D14018903
fbshipit-source-id: fd0ce2da0e11954e683385402738c701045e727c
Summary: All our C++ Fabric tests are cross-platform, so it makes sense to run them for all platforms (especially because platform may behaive differently).
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D13984574
fbshipit-source-id: e384c03c7f9839be38a1910e04ba2f7725abc378
Summary:
`Better` is a trivial collection of basic tools borrowed from other low-level general purpose libraries (like Folly, Abseil or Boost). The main goals of Better:
- Make the codebase more portable;
- Make the dependency list explicit (by decoupling it as a dependency list of Better);
- Make relying on modern C++ patterns and tools in code simple and easy.
- Make executing experiments with different dependencies easier.
As a first example usage, this diff replaces std::unordered_map with an efficient one from folly on the one of the hottest paths.
Reviewed By: JoshuaGross
Differential Revision: D13944565
fbshipit-source-id: 5fa2c4abe6c17f7361eddcc25f968b6440d5d9db
Summary:
Our long-term plan is to completely illuminate `jsi::Value`-to-`folly::dynamic` serialization step in prop parsing process improving performance and memory pressure. At the same time, we don't want to introduce a hard dependency in application code to JSI because it exposes direct access to VM and prevents parsing some data that come *NOT* from JSVM.
RawValue is an extremely light-weight (hopefully fully optimized-out) abstraction that provides limited JSON-like and C++-idiomatic interface.
The current particular implementation is still using `folly::dynamic` inside, but I have fully JSI-powered one which will replace the current one right after we figure out how to deal with folly::dynamic-specific callsites. Or we can implement RawValue in a hybrid manner if a code-size implication of that will be minimal.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D13962466
fbshipit-source-id: e848522fd242f21e9e771773f2103f1c1d9d7f21
Summary: Nothing really changed; the change is only to better express an original intent.
Reviewed By: JoshuaGross
Differential Revision: D13962464
fbshipit-source-id: f385db8ba8662f2150181e47fc6a2a981f809e96
Summary: Don't use shared_ptr in this case, it's not needed.
Reviewed By: shergin
Differential Revision: D13965413
fbshipit-source-id: ec98c13f53c7d558a0cb68cea0f97568dd202cd8
Summary: The biggest change is that (1) the image proxy/observer code from the Image component has been generalized, (2) the four image props for the Slider component are fully supported, (3) a handful of props that were ignored or buggy on iOS now perform as expected.
Reviewed By: shergin
Differential Revision: D13954892
fbshipit-source-id: bec8ad3407c39a1cb186d9541a73b509dccc92ce
Summary:
This diff adds performance loggers for Fabric in Android to be able to compare current version or RN with Fabric
This is the summary of Points and Annotations:
- **UIManager_CommitStart**: time that React starts the commit (react tree is ready to start rendering in native)
- **UIManager_LayoutTime**: this is the time it takes to calculate layout in yoga
- **UIManager_FabricFinishTransactionTime**: Time it takes transform "C++ mutationInstructions" into "Java MountItems" and cross boundaries from C++ to Java (including serialization of data) (THIS IS ONLY FABRIC)
- **UIManager_DispatchViewUpdates**: time right before RN moves the mount operations to the Queue that is going to be processed in the next tick UI thread
- **UIManager_BatchRunStart**: time right before the mountItems are going to be process in the UI Thread
- **UIManager_BatchedExecutionTime**: time it took to run batched mountItems (usually layout and prop updates on views)
- **UIManager_NonBatchedExecutionTime**: time it took to run non-batched mountItems (usually creation of views)
Reviewed By: fkgozali
Differential Revision: D13838337
fbshipit-source-id: 0a707619829e7d95ce94d9305ff434d1224afc46
Summary: This is a temporary change to measure production data
Reviewed By: fkgozali
Differential Revision: D13906807
fbshipit-source-id: 2a2f71aa379c4aca63c7bb4a9644704f713cb088
Summary: The goal is to be able to use MobileConfig params inside of core React Native C++ code. This works on Catalyst iOS now and Wilde; need to add support for FB4A and Catalyst Android.
Reviewed By: fkgozali
Differential Revision: D13883007
fbshipit-source-id: 115fe6cc884d2a0b9ca26dadf867a5f0ae99f262
Summary: Use a folly LRU implementation to cache results of ParagraphShadowNode::measure, which Yoga asks for repeatedly. Should have a substantial speed improvement on Android and iOS, or at least that's the dream.
Reviewed By: mdvacca
Differential Revision: D13795808
fbshipit-source-id: 5716af0fe0517a72716e48113c8125bb788735d7
Summary: Folly promises/futures have been replaced by an observer model which keeps track of loading state. This resolves at least one crash that I can no longer repro and simplifies the code a bit (IMO).
Reviewed By: shergin
Differential Revision: D13743393
fbshipit-source-id: 2b650841525db98b2f67add85f2097f24259c6cf
Summary: Previously we had a single `commit` function that accepts an argument that enables auto-retry mechanism. As Spencer pointed out, that wasn't so useful and clear. So, I split the method into two with more expressive names.
Reviewed By: sahrens
Differential Revision: D13681594
fbshipit-source-id: 529f729d597206e9a0ac940f005a1569a9bef707
Summary:
I read my code and one thing stroke me: What if the same event emitter dispatches several events during the same event beat? In the previous implementation, only the first one will be delivered because the first `release` call consumes stored strong reference.
So, I changed the API a bit to make this more explicit: we have `retain` & `release` methods and we have a getter that works (multiple times) only if the object was successfully retained.
Reviewed By: sahrens
Differential Revision: D13668147
fbshipit-source-id: c00af5f15bc7e30aa704d46bd23584b918b6f75a
Summary: This diff implements a new algorithm that effectively marks `EventEmitter`s enabled or disabled. The previous implementation relied on a list of mutation instructions whereas the new one analyzes the shadow trees. The mutations-based approach didn't work well because mutations describe `ShadowView`s whereas some `ShadowNode`s are simply not views (like VirtualText), but we have to enable/disable them anyway.
Reviewed By: sahrens
Differential Revision: D13642594
fbshipit-source-id: 12169e11d5685e50bcd0d8c410498c594df744b4
Summary: See the diff for more details.
Reviewed By: sahrens
Differential Revision: D13642593
fbshipit-source-id: 8bdcc91bcc2ea1e4093bcac03d87167b8901cbb4
Summary: The mutex is a "leaf" mutex, so the code cannot reenter that, so there is no reason to be a recursive one.
Reviewed By: sahrens
Differential Revision: D13642592
fbshipit-source-id: 0fb64d3405c5d3408251dc983c186f6747bc6ee2
Summary:
EventTargetWrapper and EventTarget were merged into one class that controls an `instanceHandle` reference and extracting a strong reference to it.
This diff also decouples the operation of retaining a strong reference (with checking a `enabled` flag) from actual usage of this reference. This allows to wrap into a mutex only first part of this process and avoid possible deadlocks.
Reviewed By: sahrens
Differential Revision: D13616382
fbshipit-source-id: 9907bc12047386fcf027929ae2ae41c0b727cd06
Summary:
For some time we have had a nit trick inside `EventEmitter`: When event emitter is disabled, we switch to store `EventTarget` as a weak pointer instead of complete deleting it. Given some unpredictability of JS GC, it gave us some time to restore the pointer in case if we increment the counter right after decrementing this. Apparently, we found that it doesn't always work (obviously, sigh...), so we implement the `enableCounter_` and left this feature as it is assuming that it can nice optimization that illuminates some unnecessary constraints.
But, apparently, it's actually harmful, assuming that `jsi::WeakObject` (that thing that we use to refer to actual JS target) actually has "unsafe unretained" (not "weak") semantic.
So, we have to remove this.
This change will be particularly important for coming diffs in the stack.
Reviewed By: mdvacca
Differential Revision: D13324480
fbshipit-source-id: 4c4da2984dc6a36b94b564bc9eee144142b430b0
Summary: Because it was not idiomatic anyway. There is no point having a pointer to RootShadowNode without any guarantees that it's the current one. Using `commit` method makes this concern explicit.
Reviewed By: sahrens
Differential Revision: D13615363
fbshipit-source-id: f71ffc3c55dbdc69624933eb8b92334ed793c794
Summary:
Removing additional complexity from ShadowTree should help with maintainability. Now, this class is "tricky", but short at least.
With new `commit` API, it's much more simple and expected this way.
Reviewed By: sahrens
Differential Revision: D13615365
fbshipit-source-id: 1fe851c1a2d3bdc7ac2f4a570cf0170eae3c4c67
Summary: Now it's parts of RootShadowNode and Scheduler.
Reviewed By: sahrens
Differential Revision: D13615364
fbshipit-source-id: 13dbea1e69ef51b2679101915c01c6be7e15d859
Summary:
Instead of the whole family of commit* and complete* methods, now we have one single `commit` method which performs pre- and post-commit operations and swap pointers in a thread-safe manner. The `commit` operation is also exposing `revision` number and allows perform multiple commit attempts.
`completeByReplacingShadowNode`, `measure` and `constraintLayout` are also going away to RootShadowNode class in the next commits.
Why?
* Nicer API;
* No more recursive_mutex, no more problems with thread jumps;
* All mutex locks are now leaf-locks, so no more deadlocks possible;
* Exposing `revision` should help with debugging races.
Reviewed By: sahrens
Differential Revision: D13613942
fbshipit-source-id: 94e797d2f7860717847e823b5d97c4f7b35f08df
Summary: This diff open sources Fabric Android implementation and it extracts ComponentDescriptorFactory into a function that can be "injected" per application
Reviewed By: shergin
Differential Revision: D13616172
fbshipit-source-id: 7b7a6461216740b5a1ad5ebbead9e37de4570221
Summary: We are now generating the native cpp files for Switch via Buck. Deleting the hand written files and switching over.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D13666672
fbshipit-source-id: 72cf6f6af9374511f2742f8f0d996fa52e1bff5b
Summary:
@public
Removes all `YG...Count` macros for enums and replaces them with `facebook::yoga::enums::count<YG...>()`.
This removes the need to manually maintain enum counts.
Same as D13597449, working around a defect in clang < 3.9
Reviewed By: amir-shalem
Differential Revision: D13634622
fbshipit-source-id: 344dc70e167b0caf746fe396cedd200f54e52219
Summary:
@public
Removes all `YG...Count` macros for enums and replaces them with `facebook::yoga::enums::count<YG...>()`.
This removes the need to manually maintain enum counts.
Reviewed By: shergin
Differential Revision: D13597449
fbshipit-source-id: edcee225ada4058e94f3a727246763e3cc45873d
Summary: This diff ensures that the 'measure' method in TextLayoutManager is memoized using a static variable.
Reviewed By: fkgozali
Differential Revision: D13585508
fbshipit-source-id: 9275a4d193b8abb0c3aaffd5a5535234717475e1
Summary:
This diff fixes a race condition that was detected on "Marketplace You" production test for Android.
The race condition happens when the method ShadowTree::complete is executed concurrently from two threads (in this case this is called from Scheduller::constraintSurfaceLayout and Scheduler::uiManagerDidFinishTransaction), based on the order of execution this bug makes MountViewItems to be dispatched to the UI in the wrong order.
The root cause of the bug is in the method:
```
bool ShadowTree::complete(
const SharedRootShadowNode &oldRootShadowNode,
const UnsharedRootShadowNode &newRootShadowNode) const {
newRootShadowNode->layout();
newRootShadowNode->sealRecursive();
auto mutations =
calculateShadowViewMutations(*oldRootShadowNode, *newRootShadowNode);
if (!commit(oldRootShadowNode, newRootShadowNode, mutations)) {
return false;
}
emitLayoutEvents(mutations);
if (delegate_) {
delegate_->shadowTreeDidCommit(*this, mutations);
}
return true;
}
```
Notes:
- the commit method is guarded by the commitMutex_
- the shadowTreeDidCommit() method dispatches mutations instructions to the platform side.
- If there are two threads running concurrently, there is no guarantee that the shadowTreeDidCommit() is going to be called in the correct order.
The solution is to include the execution to shadowTreeDidCommit() in the same commitMutex_
Possible solutions:
1 - move the commitMutex_ out of the commit method (to the completeMethod)
2 - synchronize the call to complete method() - this is the implemented solution.
I chose this solution to make it consistent with the way Scheduler::constraintSurfaceLayout is implemented (https://fburl.com/8l49no5x)
This mechanism is very likely to change in the refactor of threading mechanism that Valentin Shergin is going to be working on January.
I would like to land this, so we can fix this bug and run another experiment in production as soon as possible.
Reviewed By: sahrens
Differential Revision: D13535587
fbshipit-source-id: bedd4d85f5569ab3733c302d1328aa48017bcaad
Summary:
shergin mentioned that he'd like to move away from RTTI a bit and use explicit key strings for context container instances rather than relying on the `typeid`, so this does this.
We also fatal with a useful error message if we get a collision, rather than failing silently.
Reviewed By: shergin
Differential Revision: D13384308
fbshipit-source-id: 0b06d7555b082be89e8f130c23e94be99749a7a3
Summary: We need a way for different apps to inject dependencies or additional functionality into Fabric - ReactNativeConfig might be a special case, but I think this could clean up it's integration nicely, and I'm using this for a uitemplate cache system so we can use CompactDisk or other storage systems for caching depending on the app.
Reviewed By: mdvacca
Differential Revision: D13407287
fbshipit-source-id: 45481908434e6235850aa4d2d6b2bfb936a23be7
Summary:
@public
The storage format of `YGValue` in `YGStyle` is an implementation detail that is going to change soon. It is only guaranteed to be assignable from, and castable to `YGValue`.
Here, we remove tight coupling from the actual implementation in React Native.
Reviewed By: shergin
Differential Revision: D13465113
fbshipit-source-id: 41dfcb90c2a1cd825a6732854bf84d4c3318d835
Summary:
@public
When switching to `CompactValue`, casting edges or dimensions to `std::array<YGValue, ...>` will do actual work.
In order to avoid that from happening implicitely, we remove the casting operator.
Reviewed By: SidharthGuglani
Differential Revision: D13464292
fbshipit-source-id: 217065b001a63cfa8adde715063682c583007a4d
Summary:
This diff fixes a style property that was incorrectly mapped as `textDecorationLineType` in Fabric
This was correctly mapped in classic here: diffusion/FBS/browse/master/xplat/js/react-native-github/Libraries/Text/BaseText/RCTBaseTextViewManager.m;10b92f1847cdec8a3f0a996f218989766516f805$48
Reviewed By: mdvacca
Differential Revision: D13443921
fbshipit-source-id: 7fafaf2492d8c3b938f2e433a983303958e5c578
Summary:
@public
Replace `YGFloatOptional::getValue()` with `YGFloatOptional::unwrap()`.
`YGFloatOptional::getValue()` has the unfortunate property of calling `std::exit` if the wrapped value is undefined.
Here, we eliminate the method, and just call `.unwrap()` everywhere.
Reviewed By: shergin
Differential Revision: D13439608
fbshipit-source-id: 5ae82b170537d0a10c301412567a7a66fd50bab4
Summary:
@public
`YGFloatOptional::getValue()` has the unfortunate property of calling `std::exit` if the wrapped value is undefined.
That forces `x.isUndefined() ? fallback : x.getValue()` as access pattern.
Here, we replace that by introducing `YGFloatOptional::orElse(float)` which encapsulates that pattern. Other additions are `orElseGet([] { … })` and some extra operators.
Reviewed By: SidharthGuglani
Differential Revision: D13209152
fbshipit-source-id: 4e5deceaaaaf8eaed44846a8c152cc8b235e815c
Summary:
In some setup, buck cxx test for android runs with `NDEBUG` set, hence we can't call debug symbols in the test cases. So guard those callsites with `#ifndef NDEBUG`.
Also, some dependencies for this test target depend on Android specific symbols, so we have to mark it as instrumentation test for now (FB-specific).
Reviewed By: sahrens
Differential Revision: D13337637
fbshipit-source-id: 02ff152df9937f2b0b8596f53789cdee8ee8a539
Summary: For configuration purpose, pass down config object from the hosting app and use it in UITemplateProcessor.
Reviewed By: sahrens
Differential Revision: D13290322
fbshipit-source-id: 8bb6d7f5a3f977b873e548e15603259876b46dc8
Summary:
`ShadowTree` class is already thread-safe, so we don't need to (we should not) guard concurent access to it.
We should guard concurrent write-access to the collection of them though.
Note that unordered_map is "thread-compatible" collection, so concurrent reading access is okay.
Reviewed By: mdvacca
Differential Revision: D13269745
fbshipit-source-id: 4779626018da0e42b81a835e538f6c1d1a8e25f7
Summary: Trivial diff that cleans up measure function in C++ and Android
Reviewed By: shergin
Differential Revision: D13200340
fbshipit-source-id: 6c0888439640241cdedf514898a1ba3dac231d6a
Summary: Pretty straight-forward migration to using `JSI` instead of `folly::dynamic` in `TouchEventEmitter`.
Reviewed By: sahrens
Differential Revision: D13123042
fbshipit-source-id: 594b89b6e3986d6a04846194701e3a727b152cec
Summary: Pretty straight-forward migration to using `JSI` instead of `folly::dynamic` in `ScrollViewEventEmitter`.
Reviewed By: sahrens
Differential Revision: D13123049
fbshipit-source-id: 2839976d0119c48fa2538dbaa53afbc24982c598
Summary: Pretty straight-forward migration to using `JSI` instead of `folly::dynamic` in ViewEventEmitter.
Reviewed By: sahrens
Differential Revision: D13123048
fbshipit-source-id: 3c323912d3e65b684f99df6cda99c785876164af
Summary: Pretty straight-forward migration to using `JSI` instead of `folly::dynamic` in SwitchEventEmitter.
Reviewed By: sahrens
Differential Revision: D13123046
fbshipit-source-id: f2e4905a96191540ceec633bae1871c93be724db
Summary:
Now the event delive pipeline supports `JSI::Value`-based payload. Instead of passing `folly::dynamic`, now we are passing `std::function<jsi::Value(jsi::Runtime &runtime)>` as factory that can build a `JSI::Value` with given `jsi::Runtime` and any captured data.
The old (now legacy) way of calling `EventEmitter::dispatchEvent(..., const folly::dynamic &payload, ...)` is also supported.
Reviewed By: sahrens
Differential Revision: D13123043
fbshipit-source-id: d65348bb215013042abb2fcfe5083a8c697333d0
Summary: Now instead of passing `reactTag` through the whole event pipeline, we store it inside `EventTargetWrapper` (and it does not leave `UIManagerBinding`). It helps with reducing the complexity of `EventEmitter` and will help us in migrating to JSI.
Reviewed By: sahrens
Differential Revision: D13123045
fbshipit-source-id: aa9ee94d5660ff3090369c1e55cf748d2e72b987
Summary:
That's generally better because:
* Avoids exposing ShadowNode to mounting layer;
* Enables hashing and comparing the AttributedString based on actual meaningful data (not on just a pointer to ShadowNode).
Reviewed By: mdvacca
Differential Revision: D13205230
fbshipit-source-id: 7b79c1aad97b10d81e3faa10408be61b74f815cf
Summary: Trivial. We need this for future use as part of AttributedString's hash.
Reviewed By: mdvacca
Differential Revision: D13205231
fbshipit-source-id: 14a3decae72741030284a30abdb936616bafb3fe
Summary:
ShadowView, ShadowViewMutation, and Differentiator were decoupled to separate module.
That enables us to use ShadowView more widely without facing a circular dependency problem.
Reviewed By: mdvacca
Differential Revision: D13205229
fbshipit-source-id: 7373864bf153a7813c2f97edb263a41454ce0b88
Summary: Pretty straightforward wiring UIManager and the new feature in ShadowTree: we get the node, clone with the new props and then replace this.
Reviewed By: sahrens
Differential Revision: D13114788
fbshipit-source-id: 3a34fb879f3ec564c26278034a19b88518302de8
Summary: This method is the core of the future features: `setNativeProps` and `LocalState`.
Reviewed By: sahrens
Differential Revision: D13114789
fbshipit-source-id: 2138496c43c171fe27784b1959d86d6eec4638ee
Summary:
Previously, we stored a pointer to ShadowNode inside NSAttributedString's attributes to make possible retrieving an EventEmitter associated with some text fragment.
That worked fine besides only one caveat: the internal implementation of NSAttributedString is quite strange and that causes a memory leak. Because of some reason, NSAttributedString does not release stored attributes after own destruction (maybe OS uses some kind of caching).
So, now, instead of storing a strong pointer to ShadowNode inside NSAttributedString, we store a weak pointer to EventEmitter. Storing a weak pointer is okay because a desired lifetime of EventEmitter is guaranteed by LocalData stored inside a View. Storing a weak EventEmitter instead of weak ShadowNode will also help us with migration to ShadowView (we cannot store ShadowView weakly because it's a stack allocated object).
Reviewed By: sahrens
Differential Revision: D13196886
fbshipit-source-id: f8714e4b3709765629d6456edf0c635bf5f7c53b
Summary:
Consider this:
* ParagraphShadowNode retains LocalData,
* LocalData contains AttributedString,
* AttributedString contains Fragments,
* Fragment can contain a pointer to parent shadow node, so it can be the ParagraphShadowNode.
In this case it's a retain cycle.
We actually don't need to store pointers to not TextShadowNodes, so we don't now.
Later, after we fully migrate to ShadowView, we can remove this condition because it will become harmless.
Reviewed By: sahrens
Differential Revision: D13196885
fbshipit-source-id: d386ce0a067df0a72e6619d62d56038aaf80eccb
Summary:
Apparently, the previous behavior brings more problems than some *possible-in-the-future* features and flexibility.
The new model allows us to easily implement "nested text" feature.
(We temporary hope the old behavious for Android only for compatibility reasons.)
Reviewed By: mdvacca
Differential Revision: D13176277
fbshipit-source-id: 01f7bfb3c2e70cc89d76ecb78add016ee91cbd63
Summary:
This diff changes the method to calculate the hash of an AttributedString (removing shadowNode and parentShadowNode from it).
This is necessary becuase otherwise hashcode of clonned parent keep changing in every state change, when the text doesnt change
With this change we are able to cache spannables in android properly
Reviewed By: shergin
Differential Revision: D13189110
fbshipit-source-id: c1f7372809ce98a5b4d091485cc15281a4ab5e1e
Summary:
Removing two props that are not currently used
They are being set as quiet_NaN in C++ and this brings problems in the Android side
Reviewed By: shergin
Differential Revision: D13188600
fbshipit-source-id: e8412497a80300cfbc3770b829e9633206aaf427
Summary: In this diff we expose the text local data hash to android, this will be used in the future to cache metadata when rendering text in android
Reviewed By: shergin
Differential Revision: D13161873
fbshipit-source-id: cd13a4beba75a3fe62ac9ff3def26f88e874834b
Summary: Simple diff that adds a systrace to start measuring the calculation of Yoga layout() in Fabric
Reviewed By: shergin
Differential Revision: D13124641
fbshipit-source-id: 6bd03e9f56524221f5d91606ffde50253673c1bb
Summary: This diff refactors the types used when Yoga requires to measure the size of a View in C++
Reviewed By: shergin
Differential Revision: D13124086
fbshipit-source-id: 89dfe80bb41b4fb2eaba84af630d52ef2509b1e1
Summary: Now we don't update `LocalData` for `ParagraphShadowNode` if the attributed string hasn't changed.
Reviewed By: mdvacca
Differential Revision: D13160128
fbshipit-source-id: 6ffe76ad187452fa37ba36a132b885cbcedfd1d3
Summary: This method is underlying infra for all `measure`-like methods exposed to JavaScript.
Reviewed By: mdvacca
Differential Revision: D13036553
fbshipit-source-id: cb5135f1db97ec8144b31a24ee4fb9f5d61f0df1
Summary: The diff adds a pointer to ShadowTreeRegistry to UIManager which enables the possibility of implementing ShadowTree mutating and inspecting methods like `setNativeProps` and `getRelativeLayoutMetrics`.
Reviewed By: mdvacca
Differential Revision: D13036549
fbshipit-source-id: 5ed1252d84c8dd895fe0e6e8cc71afbaa9dab4b7
Summary:
Why do we need a dedicated registry class?
* We need to simplify registry-related logic in Scheduler.
* We need to couple threading aspect of the registry with the registry itself, otherwise it's not clear why exactly we acquire the mutex. We also should not acquire the mutex in a per-method way (as we did before), because it's incorrect and misleading (only lines that access the registry should by protected).
* We need to have a way to share the registry with other classes (e.g. UIManager) without passing a reference to the whole Scheduler.
Reviewed By: mdvacca
Differential Revision: D13036550
fbshipit-source-id: 644da910e823666c586834a3da2b4cdcb90eebb2
Summary: Trivial. Those are not used.
Reviewed By: fkgozali
Differential Revision: D13017883
fbshipit-source-id: cf285e537eb85c8fca6852f7c03a5ef661b85757
Summary:
The generic method that returns relative (to some specified node) layout metrics.
We need this as a building block to implement `UIManager.measure*()` methods family.
Reviewed By: mdvacca
Differential Revision: D12932549
fbshipit-source-id: 79064551d32b0cd40e72dc87d0ed194e3779ba29
Valentin Shergin
ericlewis
Joshua Gross
Rick Hanlon
Marco Zandonadi
empyrical
David Vacca
Yedidya Feldblum
Spencer Ahrens
David Aurelio
Héctor Ramos
Jonathan Kim
zhongwuzw
Eli White
Amir Shalem
Taras Tsugrii
Doug Russell
Kevin Gozali