react-native-reanimated icon indicating copy to clipboard operation
react-native-reanimated copied to clipboard

Performance degradation with large objects and arrays

Open marcocaldera opened this issue 4 months ago • 9 comments

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:

  • Skia v0.1.196 (Note: Starting from v0.1.197, Skia shared values also show performance slowdowns, potentially attributed to this commit: link)
  • Reanimated v3.6.1 with a traditional array shared value.
  • Reanimated v3.6.1 utilizing JSON.parse/stringify to store the array within a shared value.

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:

  • Skia performs with a smooth 60 FPS for both UI and JS threads.
  • Reanimated, using an array as a shared value, runs at 40 UI FPS and 30 JS FPS.
  • Reanimated, using JSON.stringify/parse with an array, achieves 40 UI FPS and 60 JS FPS.

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

marcocaldera avatar Mar 11 '24 20:03 marcocaldera

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

piaskowyk avatar Mar 14 '24 10:03 piaskowyk

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

chrfalch avatar Mar 18 '24 13:03 chrfalch

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 avatar Mar 22 '24 14:03 tomekzaw

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

tomekzaw avatar Mar 22 '24 14:03 tomekzaw

@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

ranaavneet avatar Mar 22 '24 18:03 ranaavneet

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

tomekzaw avatar Mar 23 '24 17:03 tomekzaw

@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:

  • reanimated 3.8.1
  • using an array buffer

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.

marcocaldera avatar Mar 27 '24 10:03 marcocaldera

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

tomekzaw avatar Mar 27 '24 11:03 tomekzaw

@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 :)

chrfalch avatar Apr 02 '24 06:04 chrfalch