Optimize Shadow Tree cloning

[bartlomiejbloniarz]

Summary

This PR optimizes the algorithm used to clone the Shadow Tree used in ReanimatedCommitHook and performOperations.

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:

1. calculate the path from the affected ShadowNode to the root using ShadowNodeFamily::getAncestors
2. traverse the path upwards cloning all the nodes up to the root

This way we unfortunately clone some ShadowNodes multiple times. For example for a batch of size n we will clone the root node n 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 returns true 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:

1. calculate the subtree of the ShadowTree that contains all the nodes that we want to update
2. traverse the ShadowTree and clone nodes (that belong to the subtree) in the (reversed) topological order

By calculating the subtree first we ensure that in the second phase:

1. we traverse only nodes that absolutely have to be traversed
2. we clone only nodes that absolutely have to be cloned
3. we clone every node at most once

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 the parent field in ShadowNodeFamily is private, so traversing the tree upwards is only possible through getAncestors). 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:

Phone	Example size	Before [FPS]	After [FPS]
iPhone 12 mini	200	30-40	60
One+ A6	100	10-20	30-40
iPhone 15 Pro	250	30-40	120
Samsung Galaxy S23	100	55-70	120

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).

Example size	Before [ms]	After [ms]	Before/After
1	1.95	2.1	0.92
20	2.4	2.1	1.14
100	5.3	2.3	2.3
250	22	4	5.5
500	77	7.7	10

Test plan

Check the behavior of examples in the FabricExample app. Verify that heavy examples have improved, while simpler examples have not regressed.

[ReactorCSA]

Thanks for this great library!!!
My question may be stupid but anyways, will this fix also improve performance on Paper architecture?
Thanks in advance :)

[bartlomiejbloniarz]

Hi @ReactorCSA!
This optimization is only for the New Architecture, since Shadow Tree is only a New Architecture concept.

[janicduplessis]

@bartlomiejbloniarz I've tested this in my app and it causes a crash on screens that use @gorhom/bottom-sheet. Sorry I don't have an exact repro, but here's the crash report I get.

[bartlomiejbloniarz]

@janicduplessis Thank you for testing this PR 🙏

[bartlomiejbloniarz]

@janicduplessis could you test your app again? I changed the implementation so that we don't allocate RawProps on the heap (sadly @tomekzaw was right 😭). From my tests @gorhom/bottom-sheet doesn't crash anymore, but it would be cool to see if your app is fine now 🙏

[bartlomiejbloniarz]

The issue wasn't caused by the usage of RawProps on its own. It was actually due to using the same RawProps multiple times. This happened because the ChildMap was populated wrongly (i.e. my skill issue). I decided to remove the abstraction and go back to using RawProps.

[mrousavy]

A bit unrelated, but looking at this code I can see that you can access the props of a component as jsi::Values - how did you get to that point?

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 jsi::Values...

[janicduplessis]

I’ll test it again in the next few days

[janicduplessis]

@mrousavy I think here the props as jsi::Value is new props to update the components with, not the current props of the component.

[bartlomiejbloniarz]

@mrousavy As @janicduplessis mentioned, the props here are the new props. They come from the reanimated updateProps function

[tjzel]

Looks good! 🚢 🇮🇹

[zibs]

Hey @bartlomiejbloniarz!

I just ran a git bisect between 3.14.0 and 3.15.0 where this commit is the one that causes an issue in my app: a4c5c5d39ac27c5d04f825af30f172ab8087844d is the first bad commit.

My issue occurs on bridgeless enabled + fabric

I'm attaching two videos of the issue, that seems to be related to custom scroll bars and Safe Areas, although it doesn't matter which version the react-native-safe-area-context is.

You can see that in the first video, 3.14.0, the custom scroll bar/safe areas work fine.

Screen.Recording.2024-11-08.at.9.47.34.AM.mov

You can see that in the next video, 3.15.0 (a4c5c5d39ac27c5d04f825af30f172ab8087844d is the first bad commit), the custom scroll bar/safe areas doesn't work.

Screen.Recording.2024-11-08.at.9.59.32.AM.mov

Here is the most relevant code:

    const onScrollHandler = useAnimatedScrollHandler(event => {
      if (onScroll) {
        runOnJS(onScroll)({nativeEvent: event});
      }
      let newOffset =
        ((event.contentOffset.y + largeTitleOffsetForScrollIndicator) /
          completeScrollbarHeight) *
        (difference + scrollIndicatorSize);

      if (newOffset < 0) {
        newOffset = 0;
      }

      if (newOffset > difference) {
        newOffset = difference;
      }

      scrollIndicatorOffset.value = newOffset;
    });

    const scrollStyle = useAnimatedStyle(() => {
      return {
        height: scrollIndicatorSize - headerOffsetForScrollIndicator,
        transform: [
          {
            translateY: scrollIndicatorOffset.value,
          },
        ],
      };
    });

And I can say that if I remove the translateY: scrollIndicatorOffset.value,, it "works" in the sense that the safe area doesn't break, but it does break my custom scrolling implementation.

Do you have any thoughts on what might be happening here? It would be very much appreciated, as this is one the last remaining things for me to enable bridgeless/fabric.

I will also say that this example is running "react-native": "0.74.5",, but issue is the same on 75 too.

[bartlomiejbloniarz]

Hi @zibs. I think that this issue is related to #6659. I need some more time to address it properly.