search

software-mansion / react-native-reanimated

React Native's Animated library reimplemented
https://docs.swmansion.com/react-native-reanimated/
MIT License
8.87k stars 1.29k forks source link

Performance degradation with large objects and arrays #5781

Open marcocaldera opened 6 months ago

marcocaldera commented 6 months ago

Description

👋 Hello everyone! I would like to open a discussion/file a possible bug on shared values.

The performance of our application significantly degrades when using shared values with large arrays, rendering the app sluggish and, in real-world scenarios, becomes unusable.

On the other side, employing a similar approach with Skia's useValue and useComputedValue (although these were lately removed in favour of the exclusive use of reanimated) showed notably better results.

To validate this, I've developed a demo project featuring three distinct implementations:

I'm sharing this problem here because Skia dropped using these hooks in their latest versions. The decrease in performance we noticed in our data-heavy app is quite big, and we would like to dive into it a bit more.

Demo example debugging iOS on iPhone 15 Pro:

https://github.com/software-mansion/react-native-reanimated/assets/93535783/db35b4f4-9bb6-4e6d-b527-e0e34efe5585

The video demonstrates:

I additionally run tests with flashlight on Android release mode (everything is available in the project demo under /apk). The results are based on a Samsung A51, a device with low performance but widely used. Here's a screenshot:

Screenshot 2024-03-11 at 18 52 05

Steps to reproduce

  1. Run the demo by following the instructions in the repo
  2. Switch between the 3 tabs with the different variations or follow testing.md to test on a physical device

Snack or a link to a repository

https://github.com/marcocaldera/reanimated-demo-project

Reanimated version

3.6.1

React Native version

0.72.11

Platforms

Android, iOS

JavaScript runtime

Hermes

Workflow

React Native

Architecture

Paper (Old Architecture)

Build type

Debug app & production bundle

Device

Real device

Device model

Samsung A51

Acknowledgements

Yes

piaskowyk commented 6 months ago

Hey! 👋 Thanks for the report and the prepared benchmarks! 😍 We will definitely take a look at them and try to make it faster!

chrfalch commented 6 months ago

I think the main issue comes from the fact that Skia implemented arrays without copying values - in Reanimated arrays are copied when transformed back and forth. The relevant code is found here:

https://github.com/software-mansion/react-native-reanimated/blob/469d708f3954e0f271618d2c0ae59f6658d76fae/Common/cpp/SharedItems/Shareables.cpp#L171-L175

tomekzaw commented 6 months ago

in Reanimated arrays are copied when transformed back and forth

That's correct, when you copy any value from let's say RN to UI runtime, they are first serialized from JS values in source runtime into runtime-agnostic C++ data structures (so-called "shareables") and re-created on the target runtime as JS values.

Skia implemented arrays without copying values

@chrfalch Could you please share more details on that? Assuming that accessing these arrays doesn't require any additional blocking synchronization, how is that possible?

tomekzaw commented 6 months ago

@marcocaldera I've just noticed that you're using Reanimated 3.6.1 – is this issue reproducible also on the latest version (currently 3.8.1)? I'm asking because in 3.7.0 we've released a huge improvement (#4300) that can positively affect the performance when shared values are updated frequently on the UI thread.

ranaavneet commented 6 months ago

@marcocaldera I've just noticed that you're using Reanimated 3.6.1 – is this issue reproducible also on the latest version (currently 3.8.1)? I'm asking because in 3.7.0 we've released a huge improvement (#4300) that can positively affect the performance when shared values are updated frequently on the UI thread.

Hey @tomekzaw isnt this conflicting the issue reported in #5816. This benchmark in this issue states the opposite of #4300

tomekzaw commented 6 months ago

@ranaavneet I'm afraid it all depends on the benchmark; as far as I remember in our initial benchmarks held by @piaskowyk we got 20% performance improvement, but obviously we can't predict all usages. Thanks for referencing #5816, we'll keep that in mind.

marcocaldera commented 6 months ago

@marcocaldera I've just noticed that you're using Reanimated 3.6.1 – is this issue reproducible also on the latest version (currently 3.8.1)? I'm asking because in 3.7.0 we've released a huge improvement (https://github.com/software-mansion/react-native-reanimated/pull/4300) that can positively affect the performance when shared values are updated frequently on the UI thread.

Interesting. I run a set of tests based on two new things:

Debug mode: https://github.com/software-mansion/react-native-reanimated/assets/93535783/e077d26b-81f3-4fb8-a0bc-d90cac63e7e4

UI thread runs now at 60 FPS everywhere. JS thread seems to suffer more on certain occasions.

Flashlight report on Samsung A51:

316602103-f932f28a-a4f9-41eb-a934-89418a3ff9f3

In the flashlight report maybe it's not super evident but the reanimated performance in terms of FPS is around 55 now (a nice improvement).

As you can see using ArrayBuffer leads to great performance.

tomekzaw commented 6 months ago

@marcocaldera That's right, ArrayBuffer is a powerful data structure. Glad we added support for it in https://github.com/software-mansion/react-native-reanimated/pull/5223 😄

We were also considering adding support for SharedArrayBuffer so that you can modify it safely from both runtimes (RN and UI), do you think it would be useful?

I suspect that the current approach is to use .modify but under the hood it calls runOnUI so the update from RN runtime is asynchronous. Instead, we could update the buffer directly (with proper synchronization).

chrfalch commented 6 months ago

@chrfalch Could you please share more details on that? Assuming that accessing these arrays doesn't require any additional blocking synchronization, how is that possible?

@tomekzaw: So sorry for the delay in getting back on this one. This feature was removed from Skia as previously stated, but the idea is still valid. Here is (from the top of my head what I did).

The array wrapper will be the owner of the array data, so when constructed it converts all JSI values to C++ values to be able to marshall back and forth on different UI runtimes. This gives us the possibility of keeping memory references to the same memory locations (as you also describe) throughout the lifetime of the array wrapper.

In addition we look at reading and writing as two very different operations - where reading is thread safe and available without locking - and writing is protected by a lock in the wrapper.

This way we get the benefit of not copying the data when moving between runtimes, and also the speed of reading without locking - and finally the necessary protection of write operations.

Hope this makes sense :)