-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize Shadow Tree cloning #6214
Optimize Shadow Tree cloning #6214
Conversation
packages/react-native-reanimated/Common/cpp/Fabric/ReanimatedCommitHook.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/Fabric/ReanimatedCommitHook.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/Fabric/ReanimatedCommitHook.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/Fabric/ShadowTreeCloner.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/Fabric/ShadowTreeCloner.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/Fabric/ShadowTreeCloner.h
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/Fabric/ShadowTreeCloner.cpp
Outdated
Show resolved
Hide resolved
Thanks for this great library!!! |
Hi @ReactorCSA! |
@bartlomiejbloniarz I've tested this in my app and it causes a crash on screens that use ![]() |
@janicduplessis Thank you for testing this PR 🙏 |
@janicduplessis could you test your app again? I changed the implementation so that we don't allocate |
The issue wasn't caused by the usage of |
packages/react-native-reanimated/Common/cpp/Fabric/ShadowTreeCloner.h
Outdated
Show resolved
Hide resolved
A bit unrelated, but looking at this code I can see that you can access the props of a component as I'm trying to build a simple component (iOS/Android) where the props layer is implemented in C++/JSI, and I manually pass down the props to ObjC/Java myself - but I couldn't figure out how to get access to raw props as |
I’ll test it again in the next few days |
@mrousavy I think here the props as jsi::Value is new props to update the components with, not the current props of the component. |
@mrousavy As @janicduplessis mentioned, the props here are the new props. They come from the reanimated updateProps function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚢 🇮🇹
Summary
This PR optimizes the algorithm used to clone the Shadow Tree used in
ReanimatedCommitHook
andperformOperations
.Current approach
The current algorithm works as follows. After receiving a batch of updates, we iterate over the list and apply the changes one by one. To apply changes we have to:
ShadowNodeFamily::getAncestors
This way we unfortunately clone some ShadowNodes multiple times. For example for a batch of size
n
we will clone the root noden
times.Cloning ShadowNodes is expensive, so we had implemented an optimization - whenever a node is unsealed we would change it in place instead of cloning it. Unfortunately this didn't work, since the
getSealed
method always returnstrue
in Production mode. This is not a bug, but the intended behavior, as sealing is only intended to help finding bugs in the Debug Mode. This still could be salvaged by memoizing which nodes were already cloned by us, but this approach still wouldn't be perfect, as modyfing nodes in place is still a heavy operation.New approach
To mitigate those issues we split the process into two phases:
By calculating the subtree first we ensure that in the second phase:
With this approach the second phase is performed in the optimal number of operations.
Limitations
The current implementation of phase one (building the subtree) is not optimal. It is implemented by simply calling
getAncestors
on every node from the batch. This is fortunately not a huge problem, because cloning had a much heavier impact on the performance. To optimize this there will have to be some changes done in RN (because theparent
field inShadowNodeFamily
is private, so traversing the tree upwards is only possible throughgetAncestors
). I hope to soon open a suitable PR.Some examples
I checked the performance of our heavier examples on some devices in the Release Mode. For the
BokehExample.tsx
the results are:I also tested through Xcode Instruments how much time does the
performOperations
function take on the same example. Tests were conducted on the iPhone simulator, but they should give an idea on the order of the number of operations this function makes (and how fast that number grows in relation to the example size).Test plan
Check the behavior of examples in the
FabricExample
app. Verify that heavy examples have improved, while simpler examples have not regressed.