Commit Graph
5 Commits
Author SHA1 Message Date
Joshua GrossandFacebook GitHub Bot 1e4d8d902d Core/Differ: detect and optimize reparenting
Summary:
# Summary

In previous diffs earlier in 2020, we made changes to detect and optimize reordering of views when the order of views changed underneath the same parent.

However, until now we have ignored reparenting and there's evidence of issues because of that. Because Fabric flattens views more aggressively, reparenting is also marginally more likely to happen.

This diff introduces a very general Reparenting detection. It will work with view flattening/unflattening, as well as tree grafting - subtrees moved to entirely different parts of the tree, not just a single
parent disappearing or reappearing because of flattening/unflattening.

There is also another consideration: previously, we were generating strictly too many Create+Delete operations that were redundant and could cause consistency issues, crashes, or bugs on platforms that do not handle that gracefully -
especially since the ordering of the Create+Delete is not guaranteed (a reparented view could be created "first" and then the differ could later issue a "delete" for the same view).

Intuition behind how it works: we know the cases where we can detect reparenting: it's when nodes are *not* matched up with another node from the other tree, and we're either trying to delete an entire subtree, or create an entire subtree. For perf reasons, we generate whatever set of operations comes first (say, we generate all the Delete and Remove instructions) and take note in the `ReparentingMetadata` data-structure that Delete and/or Remove have been performed for each tag (if ordering is different, we do the same for Create+Insert if those come first). Then if we later detect a corresponding subtree creation/deletion, we don't generate those mutations and we mark the previous mutations for deletion. This incurs some map lookup cost, but this is only wasteful for commits where a large tree is deleted and a large tree is created, without reparenting.

We may be able to improve perf further for certain edge-cases in the future.

# Why can't we solve this in JS?

Two things:

1. We certainly can avoid reparenting situations in JS, but it's trickier than before because of Fabric's view flattening logic - product engineers would have to think much harder about how to prevent reparenting in the general case.
2. In the case of specific views like BottomSheet that may crash if they're reparented, the solution is to make sure that the BottomSheet and the first child of the BottomSheet is never memoized, so that lifecycle functions and render are called more often; and that in every render, the BottomSheet manually clones its child, so that when the Views are recreated, the child of the BottomSheet has a tag and is an entirely different instance. This is certainly possible to do but feels like an onerous requirement for product teams, and it could be challenging to track down every specific BottomSheet that is memoized and/or hoist them higher in the view hierarchy so they're not reparented as often.

Reviewed By: shergin

Differential Revision: D23123575

fbshipit-source-id: 2fa7e1f026f87b6f0c60cad469a3ba85cdc234de
2020-08-15 19:20:33 -07:00
David VaccaandFacebook GitHub Bot 6486b8cffd Extend 'scheduler' module to compile in OSS
Summary:
This diff extends the 'scheduler' module to compile in OSS

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D22963705

fbshipit-source-id: ea2b6a4fce49c4d9552deb30e89fcba165cfe772
2020-08-07 11:12:02 -07:00
David VaccaandFacebook GitHub Bot 08d7b542de Create Android OSS build system for react/utils module
Summary:
This diff creates the Android OSS build system for the module react/utils

As part of this diff I also moved the module to react/utils folder

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D22877265

fbshipit-source-id: 717487aacb392d0f08530763a16a638b8021d501
2020-08-05 19:02:08 -07:00
David VaccaandFacebook GitHub Bot 8616f868d5 Create android oss build system for react/config module
Summary:
This diff creates the Android OSS build system for the module react/config

As part of this diff I also moved the module to react/config folder

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D22877264

fbshipit-source-id: 5b3c42580d2b1d73dc0abb48bcf4ff063b2f581f
2020-08-03 14:21:26 -07:00
David VaccaandFacebook GitHub Bot 3093010ea5 move fabric to ReactCommon/react/renderer
Summary:
This diff moves fabric C++ code from ReactCommon/fabric to ReactCommon/react/renderer
As part of this diff I also refactored components, codegen and callsites on CatalystApp, FB4A and venice

Script: P137350694

changelog: [internal] internal refactor

Reviewed By: fkgozali

Differential Revision: D22852139

fbshipit-source-id: f85310ba858b6afd81abfd9cbe6d70b28eca7415
2020-07-31 13:34:29 -07:00