Access Reanimated runtime only from UI thread

[Szymon20000]

Currently synchronous calls from js thread only waits for lock but still access UI runtime from js thread. That causes hermes to go for slow path as it only remembers last cached thread. More info here facebook/hermes#1572

Improvement:

• calling from js on UI thread is much faster

Downside:

• we now need to wait for UI thread even when lock is not taken (some non reanimated things runs on UI thread)

Possible alternatives:

• make hermes cache multiple threads with threads local
• create another thread and make ui also call exec sync so we don't need to wait for UI thread when lock is not taken

Summary

Test plan

[Szymon20000]

Hello @kmagiera !
I think you introduced synchronous calls so really curious what you think.

[Amurmurmur]

@kmagiera Can we please have a look at this ? really need this improvement, thank you!

[bartlomiejbloniarz]

Hi @Szymon20000! I don't think this is a good solution, as this could block the JS thread even more, whenever there is a long operation on the UI thread.

Also running our runtime on a separate thread, would resolve the caching problem, but would also come with a new set of problems. For example when we are running an animation we want to synchronously update the UI from our hermes runtime. Putting it on a separate thread, would mean that we have to schedule these updates to the UI thread, causing additional delays.

I think the best solution would be per thread caching in hermes. Our current approach is optimized for the most common use case for reanimated and before this native stack check was introduced it had the best performance. If we can get this change in Hermes, then I think there's no need to change it in reanimated.

If you need a temporary workaround I think you could try using the fact that this native stack check is behind an ifdef. So you could change it back to the old mechanism.

[Szymon20000]

Hello @bartlomiejbloniarz !
That's exactly what I mentioned in the downsides and also suggested making caching per thread in hemes but for now this pr make at least few apps faster so I think it's good temporary solution before hermes team is able to make such change.

[terrysahaidak]

I've tried this patch and here are the results. Mounting of an expensive and reanimated heavy component got much faster on a slow android device. The image is loading from disk cache so it flickers there.

Before:

PXL_20241128_111359338.mp4

After:

PXL_20241128_111538693.mp4

[bartlomiejbloniarz]

I understand that it can give a performance boost in some apps, but it can also result in performance degradation in other apps. I think that this is a huge behavior change in reanimated, that could result in new performance issues, just for other use cases. I'm not sure if this change would be a net positive for our users. I would have to first test this to ensure that we are not breaking anything.

I think that if it works for you, you should for now just patch-package it in reanimated (or disable the checking in Hermes). If we have more data that show that this is overall a better solution, we can merge this change.

[terrysahaidak]

Mounting reanimated useSharedValue is objectively much slower on android. you can't just get good performance having lots of reanimated components. This is why i more and more try to implement everything right now as a single worklet and use only makeUIMutable to drive animations, but can't be really used with RN Views this much, only via setNativeProps which has its own problems. Perhaps it's really better to have this addressed at Hermes level.

[kmagiera]

I scanned the attached issue and it seem like addressing this on hermes side is ultimately the way to go.

This does not rule out improvements that we could make on reanimated side to possible mitigate this, but what is not clear from the report is:

1. what is the difference in performance between calls from same thread and from different thread (% difference)
2. which part of the flow incur that additional cost: is this once per sync JS call, or depends on what the call actually does (how many JSI invocations are there)

As for this particular change one thing I'd be concerned the most is whether it may result in deadlocks. From concurrency perspective, locking on VM would never cause that while scheduling and waiting could. Second concerns is the granularity of tasks which is more coarse grained compared to blocks that require exclusive access to the runtime (there could be a single task that runs hundreds of such blocks, allowing resource locking approach to interleave the task in between those calls and as a consequence to simply wait shorter)

Finally, the main reason for introducing sync JS calls was to provide a way of accessing values synchronously while avoiding preemptive serialization of every single value that exists in the system. This was a huge performance boost but it relied on the sync value access to be of a relatively rare occurrence. Regardless of whether they rely on resource locking or scheduling as proposed here, they'd still block the execution in an nondeterministic way depending on how much stuff is happening. In the examples you posted I wonder, maybe this mechanism is simply misused, perhaps, even by the core reanimated code, and some sync JS calls can be simply avoided, similarily to how cancelAnimation calls relied on it and got fixed in #6564

[terrysahaidak]

The main problem of the mounting time currently is the fact that both useAnimatedStyle and useDerivedValue call their callbacks on JS thread, and if you use shared values in there (which is 99.999999% cases) you now calling .value on JS thread:

updater is the worklet you pass as callback to each hook, so it's called to calcualte the initial state:

react-native-reanimated/packages/react-native-reanimated/src/hook/useDerivedValue.ts

Line 58 in 0162804

initRef.current = makeMutable(initialUpdaterRun(updater));

react-native-reanimated/packages/react-native-reanimated/src/hook/useAnimatedStyle.ts

Line 454 in 0162804

const initialStyle = initialUpdaterRun(updater);

And .value call is sync:

react-native-reanimated/packages/react-native-reanimated/src/mutables.ts

Lines 151 to 157 in 0162804

	get value(): Value {
	checkInvalidReadDuringRender();
	const uiValueGetter = executeOnUIRuntimeSync((sv: Mutable<Value>) => {
	return sv.value;
	});
	return uiValueGetter(mutable as Mutable<Value>);
	},

This is basically what i see in my recordings with that gallery component which has lots of shared values and derived values.

[kmagiera]

I see.

Perhaps one of those ideas would make sense:

1. If it happens at mount time for initial JS-side uAS calls, there's a chance a lot of those values are actually new. I wonder if we could try using initialValues of those shared values in such a case instead of accessing their current value using sync call.
2. instead of accessing each value individually, maybe we can execute the whole initial uAS synchronously on the UI thread? This will bring down the number of sync calls, but from the issue description I'm unsure if that's really the limiting factor here.

[terrysahaidak]

Spent a bit more time testing this patch and it definitely introduced a few bugs. For example onDismiss callback form gorhom's bottom sheet is called with a huge (about 1s) delay. also, some other reanimated heavy components have some weird bugs.

[gaearon]

The main problem of the mounting time currently is the fact that both useAnimatedStyle and useDerivedValue call their callbacks on JS thread, and if you use shared values in there (which is 99.999999% cases) you now calling .value on JS thread:

+1 that this is a significant problem and needs to either be solved or prominently called out in the docs. We're currently trying to protect against this with if (!_WORKLET) early exits but this is extremely non-obvious.

[kmagiera]

Looks like the fix in Hermes landed already: facebook/hermes#1572 (comment)

Still the suggestions from my previous comment may still make sense as further optimisations.