PedroBern
refactor(container,lazy,hooks): avoid blocking JS thread by refactoring SharedValue.value access
Hey @andreialecu and @gkartalis (tagging bc I can't add reviewers), I was introducing this library to solve a UI pattern and noticed blocking JavaScript thread behavior that caused a regression of over 500ms to our initial screen render.
The flame graph above shows that the JavaScript thread is blocked by executeOnUIRuntimeSync for quite some time impacting the initial render time of a screen by over 500ms.
On closer analysis I found that the problem is caused by reading a SharedValue.value in the main JS thread. This library does this quite often in places that could avoid it. This PR refactors most of the places where it does not need to be a SharedValue while trying to not cause any regressions.
I understand there isn't unit or e2e tests for this library so I tried my best to test the Example app. I posted multiple recordings below of how it currently works with these changes.
I also wrote a detailed blogpost explaining this Reanimated blocking behavior.
You can also inspect the before and after CPU profilings, I stored them here. The CPU profile will show that before these changes there were 57 calls to executeOnUIRuntimeSync when navigating from the main menu to the FlashList example in the Example app and now there are only 14 calls.
| Before (57 calls) | After (14 calls) |
|---|---|
Demos
| One | Two |
|---|---|
| Three | Four |
|---|---|
| Five | Six |
|---|---|
| Seven | Eight |
|---|---|
Very interesting, I didn't know about this behavior of Reanimated. Is it documented anywhere?
Could it be a bug in Reanimated, maybe a recent regression? I don't remember running into an issue before or hearing about SharedValue blocking JS threads.
@piaskowyk could you kindly shed some light? See Andrei Calazan's blog post and reddit posts: https://andrei-calazans.com/posts/reanimated-blocking-js-thread/ https://www.reddit.com/r/reactnative/comments/1diy3pc/reanimated_can_block_the_main_javascript_thread/
I remember introducing SharedValue here specifically to improve performance in the past. For example to be able to do this on the UI thread:
https://github.com/PedroBern/react-native-collapsible-tab-view/commit/9378af6a0f95ce59a3eab9ca786baa05bb8039ba#diff-739a99e40f851900557fbc98026c870d8ac3c2da65ec6fc29d60c161effaf20dR240-R248
Hey @andreialecu, thanks for reviewing.
I also didn't know this until now, not sure if it's a regression since the behavior kinda of makes sense if you think about it. Any time you need to read a SharedValue.value from within the JavaScript thread it needs to bridge the value synchronously from the HostObject so that can have a non-trivial cost (anywhere from 5 to 20ms depending on device).
I want to highlight that the problem is not using SharedValue but repeatedly needing to read it from within the JS thread. The example diff you shared is a good example of the correct pattern which is to take something from the JS thread and run it in worklet to save CPU cost in the JS thread.
In the blogpost I tried to come up with a mental model of when something should be a SharedValue.
Are you using the new architecture?
With Paper/old architecture, as far as I remember, calling SharedValue.value in the past did not guarantee you'd get the actual last value. It was sometimes out of sync with the value from a worklet. Hence why I needed to create the useConvertAnimatedToValue hook, to try to ensure the value is kept in sync as much as possible.
@andreialecu note that my changes avoids this interchange of SharedValue to regular React state and solely relies on React state where possible.
@andreialecu note that my changes avoids this interchange of SharedValue to regular React state and solely relies on React state where possible.
I understand.
It's just that this transition to animated values was actually done on purpose, and it greatly improved performance. I'm trying to understand what changed recently for this to no longer be the case.
If accessing shared values from JS was indeed such performance-intensive, I think that it would've been discovered by others in the past. I find it odd that this is the first time I'm hearing about this, and I couldn't find other mentions of this potential problem anywhere else.
Reanimated has regressions some times, so it would be great to understand more about this from an upstream maintainer. Perhaps we can ping one on Twitter/X
What I think would help is if you could create a separate, minimal repro of this SharedValue issue, as a quick benchmark. Then we can perhaps run it on various reanimated versions and see if it's indeed a regression.
@andreialecu I understand the pushback. It will be helpful to hear from @piaskowyk to see if my understanding is correct.
I think the Example app is enough of a minimal repro. I pushed here a simple setup that allows us to run performance tests using Flashlight.
I ran these e2e tests with 10 iterations for main and for this branch. All tests were ran on the same Samsung a23 device (considered low end). These are the results:
At a first glance it seems like nothing has changed and this branch is just slightly better, but take a look at when the FPS starts dipping when we navigate to the FlashList example. On main the dip happens at around 3 seconds for the 10 iterations while on the refactored branch it happens around 2.5 seconds this means this branch is half a second faster to load the components and dispatch the paint to UI natively.
| FPS Dip on Main | FPS Dip on this branch |
|---|---|
Flashlight reports:
To visualize these yourself install Flashlight and run flashlight report fileone filetwo
I released this as 8.0.1-rc.0 to npm for the time being, so that we can all test it more easily.