perf: avoid calling executeOnUIRuntimeSync if shared value has initial value

[janicduplessis]

Summary

executeOnUIRuntimeSync is pretty slow, especially on low end android devices, and always comes up at the top when profiling apps, especially on older RN versions.

The idea is to use the initial value if the shared value hasn't changed yet. When the shared value changes the first time we call into JS to set the dirty flag to true and then it will fall back to slow path calling executeOnUIRuntimeSync.

This significantly reduces calls to executeOnUIRuntimeSync since most shared values are not changed before being connected to a view.

Test plan

Tested by patching this in an app with heavy reanimated usage and making sure animations still work properly.

Ran profiles and seen time spend in executeOnUIRuntimeSync goes from ~4000ms on RN 0.74 or ~400ms on RN 0.79 to about 0.

Before:

0.74

0.79

After:

No longer in the sample profile.

[janicduplessis]

Currently looking into making sure that in practice a race condition where the value is updated on UI thread, then read on JS before the dirty flag is passed to JS is impossible.

[tjzel]

Hi @janicduplessis, thanks for your contribution. This is an optimization we considered some time ago, but unfortunately it has some problems and we decided not to go further with it at the time. It can break the users' assumptions.

Take a look at the following example, where the user synchronously updates the shared value:

import React from 'react';
import { StyleSheet, View, Button } from 'react-native';
import { executeOnUIRuntimeSync } from 'react-native-worklets';
import Animated, {
  useAnimatedStyle,
  useSharedValue,
} from 'react-native-reanimated';

export default function Component() {
  const size = useSharedValue(100);
  const [, setRenderCount] = React.useState(0);

  const animatedStyle = useAnimatedStyle(() => ({
    width: size.value,
    height: size.value,
    backgroundColor: 'blue',
  }));

  return (
    <View style={styles.container}>
      <Animated.View style={animatedStyle} />
      <Button
        title="Toggle"
        onPress={() => {
          executeOnUIRuntimeSync(() => {
            size.value = 200;
          })();
          setRenderCount((count) => count + 1);
        }}
      />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});

With your changes this will result in the change not applied at all, because the React Native Runtime couldn't yet know that the Shared Value was dirty:

Before

ok.mov

After

bad.mov

While the example might seem a bit artificial, the same interaction could happen if a gesture changed a shared value at the same moment that a component is rerendered, losing the update from the gesture.

I think we could add a feature flag/another parameter to useSharedValue to allow this kind of behavior if the user is not afraid of flickers or lost updated but I'm not sure if we want to ship this into production for everyone.

However, with some more fast-memory features that are coming to react-native-worklets we might optimize initial reads by keeping the dirty flag in not runtime-bound state.

[janicduplessis]

Hey @tjzel, thanks for the example. I thought the value from JS was only read during the first render (https://github.com/janicduplessis/react-native-reanimated/blob/eb246ef260702e48dbcf00e55088503b2151da14/packages/react-native-reanimated/src/createAnimatedComponent/PropsFilter.tsx#L91-L96), is it being read somewhere else on updates? It is def more likely to happen if it can happen on updates too.

[janicduplessis]

As for a more reliable solution what do you think about storing the dirty flag on some shared object in c++ and have the host object return that? I think keeping that dirty flag logic might be better for now to avoid having to introduce locking on the actual value.

[tjzel]

I was prototyping Synchronizable concept a while back but decided to postpone it. It would be a perfect fit here. The flag would be held as a C++ object with a mutex.

Conceptually you'd combine your changes with:

// for reading
if(!isDirty) {
	isDirty = dirtyFlag.getBlocking();
}

// for writing
if(!isDirty) {
	isDirty = true;
 	dirtyFlag.setBlocking(true);
}

It should be significantly faster since the lock on the flag should be free majority of the time.

By the way, the issues like in the video were one of the reasons we abandoned SyncDataHolder in favor of sync reads back in the day.

[janicduplessis]

Yes, I was thinking something similar, so we have very little locking contention.

Any ideas about this point I mentionned here? #7913 (comment)

[janicduplessis]

Also any timeline on Synchronizable? Or should I look into implementing it without it and we could migrate this to use it later when its ready.

[tjzel]

Any ideas about this point I mentionned here? #7913 (comment)

I can't tell from the top of my head, but the case on the video might be that a render overwrites an updated style with a stale style. Would need to look into it more to recall all the shenanigans with style initializations etc.

Also any timeline on Synchronizable? Or should I look into implementing it without it and we could migrate this to use it later when it's ready.

It was pretty much close to completion but lacked some polish and testing. If you think it's a good idea to ship it asap then we could make it internally used in Shared Values in Reanimated 4.1.0 behind a feature flag.

[janicduplessis]

@tjzel Could you clarify how you reproduced this? I just tested on my branch in the repo fabric-example by replacing the content of apps/common-app/src/App.tsx with your example and it works on iOS, on Android it is broken both on this branch and on main.

I also double checked and the JS thread value getter is only called once on mount, so if the view is updated like in the example it doesn't has an effect.

I can still see potential for a race if the view is mounted at the exact same time as the shared value is updated, but before the callback that sets isDirty is executed. I am looking to see if I can make an aritificial repro of it.

[tjzel]

I cleaned up the repro a bit too much, here's the consistent one.

import React, { useState } from 'react';
import { StyleSheet, View, Button } from 'react-native';
import { executeOnUIRuntimeSync } from 'react-native-worklets';
import Animated, {
  useAnimatedStyle,
  useSharedValue,
} from 'react-native-reanimated';

export default function Component() {
  const size = useSharedValue(100);
  const [staticSize, setStaticSize] = useState(100);

  const animatedStyle = useAnimatedStyle(() => ({
    width: size.value,
    backgroundColor: 'blue',
  }));

  return (
    <View style={styles.container}>
      <Animated.View style={[animatedStyle, { height: staticSize }]} />
      <Button
        title="Toggle"
        onPress={() => {
          executeOnUIRuntimeSync(() => {
            size.value = 200;
          })();
          setStaticSize(size.value);
        }}
      />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});

[janicduplessis]

@tjzel I got it working with your Sychronizable implementation. I was able to repro the problem with my previous implementation using your last example, and can confirm this works now.

If you are good with getting the Sychronizable feature out for internal usage in the current state I think this would work well.

I assume we might want to split this into 2 PRs:

• Fixed conflicts and slightly simplified (supports only number, removed the extra js logic in synchronizable.ts as it wasn't working)
• Synchronizable to implement the dirty flag

[janicduplessis]

Making a patch compatible with v3 to test more in a real app and do another round of profiling.

[janicduplessis]

I get 12ms in makeSynchronizable and nothing related to getBlocking, so perf is great with this approach.

[tjzel]

@janicduplessis I brought Synchronizable to its full initial implementation, could you verify if the benchmarks still check? I took the liberty to update your PR.

Btw, after some consideration this solution is still not error-free. Not locking the runtime might still get us a stale value because shared value updates are no longer "runtime transactions".

We'd probably need a more elaborate example with i.e. gesture handler to document the behavior difference it induces.

[janicduplessis]

@tjzel Nice! Are we ok with that potential race condition? value is actually only used for the first render, so for a stale value to be read it would have to be a component mounting at the exact same time as a gesture causes an animated value to update for the first time.

That version of the Synchronizable might be a bit hard for me to test since the app is still on v3, and seems like things like SerializableJSRef do not exist. However I don't see anything there that would make it slower vs the version I tested.

[tjzel]

I wish we could be ok, but unfortunately no, this could break some user setups. My idea is to enable this optimization behind a feature flag so that the user opts into this behavior consciously - although it's hard to come up with a good name for it.

[janicduplessis]

Feature flag sounds good to me.