Patch reanimated to avoid using executeOnUIRuntimeSync

[janicduplessis]

Fixes APP-####

What changed (plus any additional context for devs)

This avoids usage of executeOnUIRuntimeSync in reanimated if the shared value has not changed yet. When an animated view is mounted, on first render, reanimated queries the value of the shared value on JS thread synchronously, but for 90% of the cases that value still has its initial value since it was created at the same time as the view. If that is the case we can avoid the expensive call and return initial value directly. Whenever the shared value changes the first time we call into JS to set the dirty flag to true and then if the shared value is connected to a view again it will fall back to slow path calling executeOnUIRuntimeSync.

On low end android this is by far the slowest thing in JS traces, since it need to lock JS thread to wait for a probably already very busy UI thread.

Edit:

Perf is a lot better after upgrading RN because of a fix in hermes facebook/hermes#1572.

Screen recordings / screenshots

Before:

0.74

0.79

After:

0.74

0.79

What to test

Test everything and look for broken animations.

[jinchung]

Launch in simulator or device for adae1e0

[jinchung]

Launch in simulator or device for 8ec1547

[christianbaroni]

I tested the build and it’s a noticeable improvement. Didn’t test everything, but I didn't see any issues where shared values are used most heavily.

This does create a small race window, right? In theory at least, where the setIsDirty invocation may have been scheduled via runOnJS, but hasn't yet run on the JS thread. If so, would be good to try to break it to understand exactly how and where that can manifest — for instance creating a shared value via useSharedValue and then before the first render completes, changing its value (via runOnUI, executeOnUIRuntimeSync) before it's read from the JS thread by useAnimatedStyle or useDerivedValue. May not happen in practice and if it does I imagine in most cases it would self correct.

[janicduplessis]

This does create a small race window, right?

Yes I think in theory it is possible, but in practice I am hoping it is not (unless using executeOnUIRuntimeSync). Good idea, let me see if I can break it.

[jinchung]

Launch in simulator or device for 95325ed

[jinchung]

Launch in simulator or device for 3bb546b

[jinchung]

Launch in simulator or device for f028ddf

[jinchung]

Launch in simulator or device for 5f2cfb8

[jinchung]

Launch in simulator or device for ec1fd08

[janicduplessis]

Been working with the reanimated team and got a version working without the possibility of race condition, using a new Synchronized value type (basically a value that can be accessed across runtimes with a lock). software-mansion/react-native-reanimated#7913, I've updated the patch to an adapted version of it that works on reanimated v3.

[jinchung]

Launch in simulator or device for 4cb902d

[WoLewicki]

After looking at the code and reading through the discussion with Reanimated team, I think there are some points to it.

• according to the discussion in reanimated repo, there still might be cases where values go out of sync, and it can be very non-deterministic, which might make it harder to reproduce/debug
• It is quite a big improvement and I saw executeOnUIRuntimeSync taking much time e.g. when opening Swap modal.
• At the same time, with 0.79, this issue is not visible that much

I think in the end it's a matter of do we prefer better performance over being sure that we won't see random visual glitches. I think we can give it to QA team, and if they don't spot anything wrong after thoroughly testing it, I'd give it a go.

[janicduplessis]

I think with this current version the possible issue is so unlikely to happen that the perf improvement is worth it. I tried to create an artificial case that trigger the issue and wasn't able to.

I will fix the conflict then I think we're good to go with this.

[WoLewicki]

Approving but let's keep potential issues in mind with this one!

[jinchung]

Launch in simulator or device for ff95cd7