Skip to content

Conversation

@hannojg
Copy link
Contributor

@hannojg hannojg commented May 29, 2025

Summary

Addresses: #7480

This PR shows a PoC of how we can sync our state from the UI thread back to JS thread / react reconciler.

Consider the following example:

  • In your app you have a complicated navigation structure and so far across multiple screen you've animated 100 views that aren't removed yet
  • On a screen you want to animate one button
  • in the cloneShadowTree method we still have the previous 100 animated views in the registry
  • We now need to clone 101 nodes, instead of just one for running the animation

The more nodes we keep and clone the slower the function gets our performance profilings have shown, which is a major performance bottleneck.

We keep all props in the props registry so that when react is performing a commit from the JS thread we don't "drop" the changes we made on the UI thread.
This mechanism now allows us to update the views/nodes on JS, so we don't need to keep all props.

As updates in fabric are batched, even if we update a lot of nodes it should result only in one react commit.

To provide a performance measurement. We have a huge list where each item is animated in, so they are added to the prop registry. Scrolling became slow over time.
For this screen we had a 37.43% jank frame rate. With this change the jank frame rate dropped to 19.99%.
As a reference, when having completely disabling the commit/mount hooks the jank frame rate is 16.70%.

(I don't want to dilute the scope of this description too much, but we also added back the fast update path where we update none layout props directly instead of running through the commit pipeline, and landed on a jank frame rate of 12.86%)

Things to improve in this PR

  • Currently for detecting if an animation is done in updateProps we check if the last update is more than 20 ms ago (~ 2 frames). This is really wonky and it would be better to detect this through some other mechanisms
  • I created ComponentRegistry.ts but there might be existing registries / mechanisms that I could use?
  • Some C++ cleanups

Test plan

This PoC commit shows an example for syncing our state from the UI state back to JS. This enables us to remove nodes from the prop registry.
This in turn is a performance improvement as for nodes that are not animating right now we don't need to necessarily clone those
@MatiPl01
Copy link
Member

Hey @hannojg!

Thanks for your proposal. I appreciate your effort and all the suggestions you made.

Although your suggested changes look promising, I feel that they make the current already complex approach even more complicated. We are currently working on the new approach in which we are trying to get rid of all these registries and (if possible) remove or simplify the commit and the mount hook. You can see the current work on the native-view branch, but it is still in progress and it is just in the beginning phase.

I hope that the new approach we are trying to implement would solve many issues and reduce the code complexity, which becomes unmaintainable with more registries being added.

Anyways, thanks a lot for your effort. We will consider changes you proposed and maybe implement them in this or a similar form if we don't succeed implementing the new approach (from the native-view branch).

@tomekzaw
Copy link
Member

tomekzaw commented Nov 4, 2025

@hannojg I'd like to revisit this draft PR. I've merged current main but wasn't able to push commits to this PR. Could you please merge https://github.com/software-mansion/react-native-reanimated/commits/sync-state/ branch?

Also, looks like this draft PR works only on Android because it's not possible to obtain rawProps from Props on iOS. Do you have any idea how to achieve the same thing on iOS?

@tomekzaw
Copy link
Member

tomekzaw commented Nov 7, 2025

@hannojg I've managed to revisit this draft PR. Here are my main concerns regarding the proposed changes:

  1. This draft PR doesn't work on iOS. The rawProps field is available only on Android (see Props.h: https://github.com/facebook/react-native/blob/60471cb268a70546d6ea555d25c90ca47ad3d839/packages/react-native/ReactCommon/react/renderer/core/Props.h#L63). It's hidden behind RN_SERIALIZABLE_STATE compilation macro which is supposed to be enabled only on Android. This draft PR doesn't provide any counterpart mechanism for iOS. https://github.com/facebook/react-native/blob/60471cb268a70546d6ea555d25c90ca47ad3d839/packages/react-native/ReactCommon/react/renderer/core/Props.h#L63
  2. It doesn't work for <Animated.View> inside <GestureDetector> (pretty common scenario). Turns out, GestureDetector is an animated component itself. In such case, both Animated.View and GestureDetector have the same viewTag (because they point to the same host view). Because of this, ComponentRegistry.register(viewTag, this); will first register Animated.View under some viewTag which will be then overwritten by registering GestureDetector which is one level higher in the React tree. So the style updates will be sent to GestureDetector instead of Animated.View. It can be fixed by skipping the map update if there's already a map item with the same viewTag. (Thanks to @j-piasecki and @bartlomiejbloniarz for helping me and figuring that out.)
  3. It requires additional memory to store accumulated animated styles. The global.__lastUpdateByTag map holds all style updates for all animated components on the JavaScript side. This duplicates the current data structures that we have on the C++ side and will increase the memory usage of the app.
  4. It is not safe to remove the accumulated styles in ReanimatedCommitHook because the commit may fail. If the commit fails, React Native will retry the commit. However, during the second pass, ReanimatedCommitHook won't apply the animated changes because they have already been removed from the registry. This would lead to flickering issues.
  5. The checkPropsEqual mechanism can be flaky in some edge cases. I can imagine a case that some animated component is updated as a result of React render with exactly the same values that the checkPropsEqual condition is met, leading to removing the animated styles from the registry on the C++ side before the synchronization of animated styles back to React is complete.

This means we can't merge the PR in its current state. ⚠️ We also discourage from using the changes from this draft PR in their apps, even if it seems to improve the performance, as it can lead to much more serious issues described above.

However, I'd like to confirm that the core idea of this draft PR (which is how to synchronize the animated styles back to React once the animations have settled using this.setState method in AnimatedComponent class) is correct. Inspired by this PR, I managed to submit another draft PR which uses the same technique to synchronize styles of settled animations back to React:

We already tested this draft PR on a large production app and managed to increase JS FPS from ~17 fps to ~56 fps while keeping stable 60 fps on the UI thread so we have really high hopes for this one. We also have some draft ideas on how to achieve the same effect without requiring a full React commit using the ShadowNode runtime reference update mechanism but this is yet to come as we want to land the draft PR in its current form as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants