Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize useAnimatedReaction #4466

Merged
merged 4 commits into from
May 18, 2023
Merged

Conversation

j-piasecki
Copy link
Member

Summary

Values returned by the prepare function inside useAnimatedReaction are assigned to the previous shared value. Even though it is only read from the UI thread, those values are cloned so that they can be read from the JS thread. This process turns out to be a tad problematic when dealing with large objects, as it can take a significant amount of time.

This PR sets the oneWayReadsOnly flag when creating previous to true, disabling the cloning process and reducing the performance impact of the hook.

Test plan

Here's a simple reproduction for this problem
import React from 'react';
import {SafeAreaView} from 'react-native';
import {
  Gesture,
  GestureDetector,
  GestureHandlerRootView,
} from 'react-native-gesture-handler';
import Animated, {
  useAnimatedReaction,
  useAnimatedStyle,
  useSharedValue,
} from 'react-native-reanimated';

const largeArray = new Array(1000).fill(0).map((_, i) => i);

function Bar(props: any) {
  const backgroundColor = useSharedValue('#ff0000ff');

  useAnimatedReaction(
    () => {
      return [props.isScrolling.value, largeArray];
    },
    ([isScrolling]) => {
      if (isScrolling) {
        backgroundColor.value = '#00ff00ff';
      } else {
        backgroundColor.value = '#ff0000ff';
      }
    },
  );

  const style = useAnimatedStyle(() => {
    return {
      transform: [
        {translateX: props.scrollX.value},
        {translateY: props.scrollY.value},
      ],
      backgroundColor: backgroundColor.value,
    };
  });

  return (
    <Animated.View
      style={[
        {width: 300, height: 50, backgroundColor: 'red', marginTop: 8},
        style,
      ]}
    />
  );
}

const items = new Array(50).fill(0).map((_, i) => i);

export default function App() {
  const scrollX = useSharedValue(0);
  const scrollY = useSharedValue(0);
  const isScrolling = useSharedValue(false);

  const pan = Gesture.Pan()
    .onStart(() => {
      isScrolling.value = true;
    })
    .onEnd(() => {
      isScrolling.value = false;
    })
    .onChange(e => {
      scrollX.value += e.changeX;
      scrollY.value += e.changeY;
    })
    .runOnJS(true);

  return (
    <SafeAreaView style={{flex: 1}}>
      <GestureHandlerRootView style={{flex: 1}}>
        <GestureDetector gesture={pan}>
          <Animated.View style={{flex: 1}}>
            {items.map(item => (
              <Bar
                key={item}
                scrollX={scrollX}
                scrollY={scrollY}
                isScrolling={isScrolling}
              />
            ))}
          </Animated.View>
        </GestureDetector>
      </GestureHandlerRootView>
    </SafeAreaView>
  );
}

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@j-piasecki j-piasecki added this pull request to the merge queue May 18, 2023
Merged via the queue into main with commit 4ebd011 May 18, 2023
@j-piasecki j-piasecki deleted the @jpiasecki/optimize-useAnimatedReaction branch May 18, 2023 14:35
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Values returned by the `prepare` function inside `useAnimatedReaction`
are assigned to the `previous` shared value. Even though it is only read
from the UI thread, those values are cloned so that they can be read
from the JS thread. This process turns out to be a tad problematic when
dealing with large objects, as it can take a significant amount of time.

This PR sets the `oneWayReadsOnly` flag when creating `previous` to
true, disabling the cloning process and reducing the performance impact
of the hook.

## Test plan

<details>
<summary>
Here's a simple reproduction for this problem
</summary>

```jsx
import React from 'react';
import {SafeAreaView} from 'react-native';
import {
  Gesture,
  GestureDetector,
  GestureHandlerRootView,
} from 'react-native-gesture-handler';
import Animated, {
  useAnimatedReaction,
  useAnimatedStyle,
  useSharedValue,
} from 'react-native-reanimated';

const largeArray = new Array(1000).fill(0).map((_, i) => i);

function Bar(props: any) {
  const backgroundColor = useSharedValue('#ff0000ff');

  useAnimatedReaction(
    () => {
      return [props.isScrolling.value, largeArray];
    },
    ([isScrolling]) => {
      if (isScrolling) {
        backgroundColor.value = '#00ff00ff';
      } else {
        backgroundColor.value = '#ff0000ff';
      }
    },
  );

  const style = useAnimatedStyle(() => {
    return {
      transform: [
        {translateX: props.scrollX.value},
        {translateY: props.scrollY.value},
      ],
      backgroundColor: backgroundColor.value,
    };
  });

  return (
    <Animated.View
      style={[
        {width: 300, height: 50, backgroundColor: 'red', marginTop: 8},
        style,
      ]}
    />
  );
}

const items = new Array(50).fill(0).map((_, i) => i);

export default function App() {
  const scrollX = useSharedValue(0);
  const scrollY = useSharedValue(0);
  const isScrolling = useSharedValue(false);

  const pan = Gesture.Pan()
    .onStart(() => {
      isScrolling.value = true;
    })
    .onEnd(() => {
      isScrolling.value = false;
    })
    .onChange(e => {
      scrollX.value += e.changeX;
      scrollY.value += e.changeY;
    })
    .runOnJS(true);

  return (
    <SafeAreaView style={{flex: 1}}>
      <GestureHandlerRootView style={{flex: 1}}>
        <GestureDetector gesture={pan}>
          <Animated.View style={{flex: 1}}>
            {items.map(item => (
              <Bar
                key={item}
                scrollX={scrollX}
                scrollY={scrollY}
                isScrolling={isScrolling}
              />
            ))}
          </Animated.View>
        </GestureDetector>
      </GestureHandlerRootView>
    </SafeAreaView>
  );
}

```
</details>
github-merge-queue bot pushed a commit that referenced this pull request Jun 27, 2023
…eserve value between renders (#4633)

<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

In #4466
I've replaced `useSharedValue` with `useRef` to avoid cloning saved
objects on every update. This worked really well unless, as it turned
out, there was a re-render in between the updates, in which case the
saved value would reset back to null.

I think it's because the ref object that was captured in the closure of
the worklet responsible for updating was actually a cloned object
instead of the real ref, so each update would be applied to the cloned
one. This PR reverts `useRef` change and instead passes `true` for
`oneWayReadsOnly`, simply preventing the cloning as it was originally
done in
#4466.

Fixes
#4615

## Test plan

Check out the reproduction code from the issue above.
Latropos pushed a commit that referenced this pull request Jul 3, 2023
…eserve value between renders (#4633)

<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

In #4466
I've replaced `useSharedValue` with `useRef` to avoid cloning saved
objects on every update. This worked really well unless, as it turned
out, there was a re-render in between the updates, in which case the
saved value would reset back to null.

I think it's because the ref object that was captured in the closure of
the worklet responsible for updating was actually a cloned object
instead of the real ref, so each update would be applied to the cloned
one. This PR reverts `useRef` change and instead passes `true` for
`oneWayReadsOnly`, simply preventing the cloning as it was originally
done in
#4466.

Fixes
#4615

## Test plan

Check out the reproduction code from the issue above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants