Introduce executeOnUIRuntimeSync and use it to replace Sync Data Holder #4300
Conversation
## Summary Minor changes to #4300. ## Test plan 🚀
…Data Holder (#5513) ## Summary This PR fixes some small issues from #4300 --------- Co-authored-by: Tomasz Żelawski <[email protected]>
|
Hi @tomekzaw @tjzel We currently pass
Apologies for bothering you guys during holidays! PS: Merry Christmas in advance! 🎄 |
|
@ranaavneet Today @tomekzaw was explaining to me bits of this PR in regard to performance so here's what I know.
Merry Christmas to you too! |
There was a problem hiding this comment.
I've left some minor suggestions in the comments.
Apart from this, I have some thoughts regarding the granularity of locks.
When we use the Reanimated runtime synchronously on the JS thread, we have one giant lock that wraps the call, which is nice.
On the other hand, when we use the Reanimated runtime on the UI thread, we call lock and unlock in before and after methods. This means that when we invoke the UI runtime using JSI from C++ code, we lock and unlock it multiple number of times.
Is it okay to have different levels of granularity in these two scenarios? Is there some way we can lock the Reanimated runtime once for the whole animation frame to avoid short-locking?
Based on my understanding, in this case, we require both levels of locking. The We can further discuss this during our meeting tomorrow with kmagiera. |
…ds (#5759) ## Summary Fixes #5660, a regression introduced in #4300. The aforementioned race condition happens this way: 1. An object using a `ShareableHandle` underneath (e.g. a shared value) is created. 2. This object is accessed on UI thread. 3. The initializer of this `ShareableHandle` get called on the UI thread. 4. At the same time, JS thread schedules an operation on this object (e.g. setting a value of shared value). 5. Access on UI thread forces the initialization. The condition of the if clause resolves to true and UI thread tries to access the runtime.  6. However, access from JS thread has locked the runtime and causes the UI runtime to wait. 7. Then, JS thread enters the same if clause body and initializes the whole shareable. 8. After initializing and releasing the runtime, UI thread gets unblocked. 9. However, now `initializer_` has been nulled and causes memory access issues. It's difficult to change the whole flow of locking to prevent such scenarios. Therefore we won't null the `initializer_` object anymore. However, this won't fix the problem of potential double initialization. Luckily, the code of `valueUnpacker` already prevents that with its shareable handle cache and the fact that runtime operations must be sequential. ## Test plan Run the following race condition reproduction made - you should re-run the app several times (probably up to 20ish) since it's most likely to happen when the app starts. It's also more likely to happen on Android simulators (at least for me). <details> <summary> Test code </summary> ```tsx import {useSharedValue} from 'react-native-reanimated'; import {useEffect, useState} from 'react'; const value = 666666; const Screen = () => { const sv = useSharedValue(value); useSharedValue(1); useSharedValue(2); useSharedValue(3); sv.value = value; useEffect(() => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const currentValue = sv.value; }, [sv]); return null; }; const ReanimatedCrashReproduction = () => { const [render, setRender] = useState(false); useEffect(() => { const interval = setInterval(() => setRender(r => !r), 500); return () => clearInterval(interval); }, []); return render ? <Screen /> : null; }; export default ReanimatedCrashReproduction; ``` </summary> You can do the following to see that double initialization happens still, but all is well 🚰.  Big thanks to @piaskowyk for the debugging help 🚀
## Summary Fixes `useSharedValue` type definition in docs after changes in the #4300 PR. ## Test plan No tests :)
Summary
This PR introduces a new concept of executing worklets on the UI runtime synchronously from the RN JS runtime.
It is implemented using runtime decorator that uses a lock to guard all the code that's being executed on that runtime. Thanks to this locking mechanism, we can now grab that lock on a different thread and safely execute some more methods without worrying that the runtime object will be accessed concurrently.
This new method makes opens up some new possibilities. The first one being the way we handle two-way shared values. The shared value rewrite introduced a concept of one-way shared values that are only accessed and modified from the ui runtime. We initially thought such values can be used in majority of the use cases, however, in practice it turned out that a lot of existing code already relies on reading from shared values on the RN thread. Despite the reads happening very infrequently (just a few times per shared value runtime), we still needed to serialize all the updates for a given shared value such that it can be read later on from the RN thread. With this change, we can leverage synchronous execution of the code on UI runtime and ask it to read the value on demand rather than synchronizing the value after each single update.
Test plan