Skip to content

Conversation

@j-piasecki
Copy link
Member

Description

I noticed that useGestureCallbacks still took significant time to run, despite not doing that much. I narrowed it down to the fact that it created a lot of closures for different events, while we only need two: one for JS and the other for Reanimated.

This PR refactors how the state machine is created for gestures to reduce the number of closures created to 2 per useGestureCallbacks call.

Test plan

Tested on the stress-test
import React, { Profiler, useEffect, useRef } from 'react';
import { ScrollView, StyleSheet, View } from 'react-native';
import { Gesture, GestureDetector, usePanGesture } from 'react-native-gesture-handler';
import { PerfMonitor } from 'react-native-gesture-handler/src/v3/PerfMonitor';
import Animated, {
  useAnimatedStyle,
  useSharedValue,
} from 'react-native-reanimated';

const DATA = new Array(500).fill(null).map((_, i) => `Item ${i + 1}`);

function Item() {
  const translateX = useSharedValue(0);

  const style = useAnimatedStyle(() => {
    return {
      transform: [{ translateX: translateX.value }],
    };
  });

  const pan = usePanGesture({
    disableReanimated: false,
    onUpdate: (event) => {
      'worklet';
      console.log('pan onUpdate', event.changeX);
    },
    onActivate: () => {
      'worklet';
      console.log('pan onStart', _WORKLET);
    },
    onDeactivate: () => {
      'worklet';
      console.log('pan onEnd');
    },
    onBegin: () => {
      'worklet';
      console.log('pan onBegin');
    },
    onFinalize: () => {
      'worklet';
      console.log('pan onFinalize');
    },
    onTouchesDown: () => {
      'worklet';
      console.log('pan onTouchesDown');
    },
  });

  return (
    <View style={{ height: 80, padding: 16, backgroundColor: 'gray' }}>
      <GestureDetector gesture={pan}>
        {/* <View collapsable={false} style={{opacity: 0.5}}> */}
        <Animated.View
          style={[
            { backgroundColor: 'red', height: '100%', aspectRatio: 1 },
            style,
          ]}
        />
        {/* </View> */}
      </GestureDetector>
    </View>
  );
}

function Benchmark() {
  return (
    <ScrollView
      style={{ flex: 1 }}
      contentContainerStyle={{ flexGrow: 1, gap: 8 }}>
      {DATA.map((_, index) => (
        <Item key={index} />
      ))}
    </ScrollView>
  );
}

const TIMES = 35;

export default function EmptyExample() {
  const times = useRef<number[]>([]).current;
  const [visible, setVisible] = React.useState(false);

  useEffect(() => {
    if (!visible && times.length < TIMES) {
      setTimeout(() => {
        setVisible(true);
      }, 24);
    }

    if (times.length === TIMES) {
      // calculate average, but remove highest and lowest
      const sortedTimes = [...times].sort((a, b) => a - b);
      sortedTimes.shift();
      sortedTimes.shift();
      sortedTimes.pop();
      sortedTimes.pop();
      const avgTime =
        sortedTimes.reduce((sum, time) => sum + time, 0) / sortedTimes.length;

      console.log(`Average render time: ${avgTime} ms`);

      console.log(JSON.stringify(PerfMonitor.getMeasures(), null, 2));
      PerfMonitor.clear();
    }
  }, [visible]);

  return (
    <View style={styles.container}>
      {visible && (
        <Profiler
          id="v3"
          onRender={(_id, _phase, actualDuration) => {
            times.push(actualDuration);
            setTimeout(() => {
              setVisible(false);
            }, 24);
          }}>
          <Benchmark />
        </Profiler>
      )}
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
});
Before After
Reanimated 549ms 519ms
No reanimated 472ms 460ms

event.state !== event.oldState
) {
if (event.oldState === State.ACTIVE) {
runCallback(CALLBACK_TYPE.END, callbacks, event, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not in this PR, but we should rename START, END callbacks

Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

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

If we have handleTouchEvent, maybe we should also handleUpdateEvent and handleStateChangeEvent?

Beside that looks good.

}
}

export function handleUpdate<THandlerData>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export function handleUpdate<THandlerData>(
export function handleUpdateEvent<THandlerData>(

import { TouchEventType } from '../../../TouchEventType';
import { GestureTouchEvent } from '../../../handlers/gestureHandlerCommon';

function handleStateChange<THandlerData>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function handleStateChange<THandlerData>(
function handleStateChangeEvent<THandlerData>(

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