nandorojo / moti

🐼 The React Native (+ Web) animation library, powered by Reanimated 3.
https://moti.fyi
MIT License
3.9k stars 120 forks source link

Skeleton makes navigation transitions slow by 200-400ms #251

Closed ansh closed 1 year ago

ansh commented 1 year ago

Is there an existing issue for this?

Current Behavior

When I put a Skeleton loader inside a screen and navigate to it, the app hangs for 200ms-400ms while the loader renders when comparing this to rendering null, ActivityIndicator, or any other custom loading component.

Expected Behavior

Ideally, the Skeleton loader should not add any extra time to the loading sequence, which would give a bad UX to the user. However, using Reanimated may introduce some extra time but hopefully that can be sub-100ms so provide an acceptable UX.

Steps To Reproduce

No response

Versions

- Moti: 0.21.0
- Reanimated: 2.12.0
- React Native: 0.70.5
- Expo SDK v47

Screenshots

Without Skeletons:

 LOG  Sentry Logger [log]: [Measurements] Adding measurements to transaction {
  "stall_count": {
    "value": 0,
    "unit": "none"
  },
  "stall_total_time": {
    "value": 0,
    "unit": "millisecond"
  },
  "stall_longest_time": {
    "value": 0,
    "unit": "millisecond"
  }
}
 LOG  Sentry Logger [log]: [Tracing] Finishing navigation transaction: AllFriends.
 LOG  Sentry Logger [log]: [Measurements] Adding measurements to navigation transaction AllFriends: {
  "frames_total": {
    "value": 312,
    "unit": "none"
  },
  "frames_frozen": {
    "value": 0,
    "unit": "none"
  },
  "frames_slow": {
    "value": 2,
    "unit": "none"
  }

With Skeletons:

 LOG  Sentry Logger [log]: [Measurements] Adding measurements to transaction {
  "stall_count": {
    "value": 3,
    "unit": "none"
  },
  "stall_total_time": {
    "value": 348.51123046875,
    "unit": "millisecond"
  },
  "stall_longest_time": {
    "value": 159.5732421875,
    "unit": "millisecond"
  }
}
 LOG  Sentry Logger [log]: [Tracing] Finishing navigation transaction: AllFriends.
 LOG  Sentry Logger [log]: [Measurements] Adding measurements to navigation transaction AllFriends: {
  "frames_total": {
    "value": 1043,
    "unit": "none"
  },
  "frames_frozen": {
    "value": 0,
    "unit": "none"
  },
  "frames_slow": {
    "value": 6,
    "unit": "none"
  }
}

Reproduction

Here is the Skeleton code I am using:

const Loader = () => {

  const renderSkeletonItem = () => (
    <View
      style={{
        flexDirection: "row",
        justifyContent: "space-between",
        marginBottom: 10,
        alignItems: "center",
      }}
    >
      <View style={{ flexDirection: "row", alignItems: "center" }}>
        <Skeleton height={50} width={50} radius="round" />
        <View style={{ marginLeft: 8 }}>
          <Skeleton height={16} width={124} />
          <View style={{ height: 8 }} />
          <Skeleton height={16} width={100} />
        </View>
      </View>
      <Skeleton height={35} width={91} radius="round" />
    </View>
  );

  return (
    <Skeleton.Group show={true}>
      {[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map((index) => (
        <View key={index}>
          <View
            style={{
              flexDirection: "row",
              justifyContent: "space-between",
              marginBottom: 10,
              alignItems: "center",
            }}
          >
            <View style={{ flexDirection: "row", alignItems: "center" }}>
              <Skeleton height={50} width={50} radius="round" />
              <View style={{ marginLeft: 8 }}>
                <Skeleton height={16} width={124} />
                <View style={{ height: 8 }} />
                <Skeleton height={16} width={100} />
              </View>
            </View>
            <Skeleton height={35} width={91} radius="round" />
          </View>
        </View>
      ))}
    </Skeleton.Group>
  );

  return (
    <Skeleton.Group show={false}>
      <FlatList
        contentContainerStyle={{ marginHorizontal: 16 }}
        data={[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]}
        ListHeaderComponent={<Skeleton height={48} width="100%" />}
        ListHeaderComponentStyle={{ marginVertical: 16 }}
        renderItem={renderSkeletonItem}
      />
    </Skeleton.Group>
  );
};

As you can see, I have one version that renders it with a FlatList, another one that just uses .map(). So, the issue is with Skeleton.

nandorojo commented 1 year ago

A few things:

  1. are you using native stack?
  2. what if you upgrade to reanimes v3 rc?
  3. what if you memoize the loader component?
  4. can you reduce the number of skeletons? I think the cost is coming from Reanimated shares values

I haven’t experienced this though.

ansh commented 1 year ago
  1. Yes, I am using Native Stack Navigator: import { createNativeStackNavigator } from "@react-navigation/native-stack"
  2. I tried with Reanimated 3.0.0-rc.9 and rc.8 and faced the bug of Can't find variable: jsThis when using moti image
  3. I had actually already wrapped the whole component in React.memo() when I got the numbers above. I also tried wrapping renderSkeletonItem in useMemo. There was no noticeable change in
  4. I am using 10 skeletons because that is how many items are on the screen. If I use lesser than that, it diminishes improved UX which is the benefit of using Skeleton loading. However, there is still a 10ms to 50ms delay when using only one item in Skeleton loading. Compared to 0ms when rendering a FlatList with 20 items or anything else (like an SVG)
ansh commented 1 year ago

@nandorojo Also, when you say you haven't noticed, have you tried profiling the app with Sentry or a similar tool to see the lag while switching screens? I think that is the only way to notice this issue.

I am happy to provide a simple Github repo where this problem occurs as well. It's sad that Snack doesn't support Moti anymore otherwise I used that. However, the component that I put in my original issue can be copy-pasted into any project and it is then possible to see the lag and slowdown caused by the Skeleton loaders.

nandorojo commented 1 year ago

I haven't noticed it from using it I mean. What's a good way to profile screens?

ansh commented 1 year ago

I've been using Sentry. All the logs above are from Sentry.

To be honest, 100-200ms is not that easily noticeable for something like this. But, on Android it goes upto 1400ms-2000ms for my example above and that is when I really noticed it happening badly. (Testing on a real device, production build, POCO F1).

Then, I profiled it on an iPhone simulator (200ms-400ms lag) and started noticing it. It happens when your screen isn't in-memory and you click to navigate. But, after the onPress event from the button fires, that is when the 200-400ms lag comes in play. So, essentially it takes that much time extra after the button is pressed and before the screen is navigated to. My main issues around this is a worse UX for users because they will just end up waiting around while the screen is navigating.

nandorojo commented 1 year ago

You said you’re using 10 skeletons, but isn’t it far far more than that since you’re mapping through items?

nandorojo commented 1 year ago

Can you loop through like 2 instead of that many?

nandorojo commented 1 year ago

Alternatively, one hack: hoist a single list item and try to render that same item multiple times. I wonder if that reduces memory usage at all.

ansh commented 1 year ago

Not sure what you mean by hoisting? Can you elaborate?

Btw, if I only render a single skeleton item, it still adds 50ms delay.

nandorojo commented 1 year ago

Small thing: you should add syntax highlighting to your code samples by adding tsx after the first three backticks. Easier to read.

Instead of this:

  return (
    <Skeleton.Group show={true}>
      {[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map((index) => (
        <View key={index}>
          <View
            style={{
              flexDirection: "row",
              justifyContent: "space-between",
              marginBottom: 10,
              alignItems: "center",
            }}
          >
            <View style={{ flexDirection: "row", alignItems: "center" }}>
              <Skeleton height={50} width={50} radius="round" />
              <View style={{ marginLeft: 8 }}>
                <Skeleton height={16} width={124} />
                <View style={{ height: 8 }} />
                <Skeleton height={16} width={100} />
              </View>
            </View>
            <Skeleton height={35} width={91} radius="round" />
          </View>
        </View>
      ))}
    </Skeleton.Group>
  );

What if you did this:

const item = useMemo(() => (
         <View
            style={{
              flexDirection: "row",
              justifyContent: "space-between",
              marginBottom: 10,
              alignItems: "center",
            }}
          >
            <View style={{ flexDirection: "row", alignItems: "center" }}>
              <Skeleton height={50} width={50} radius="round" />
              <View style={{ marginLeft: 8 }}>
                <Skeleton height={16} width={124} />
                <View style={{ height: 8 }} />
                <Skeleton height={16} width={100} />
              </View>
            </View>
            <Skeleton height={35} width={91} radius="round" />
          </View> 
  ), [])

return (
    <Skeleton.Group show={true}>
      {item}
      {item}
      {item}
      {item}
      {item}
      {item}
      {item}
      {item}
    </Skeleton.Group>
  );
ansh commented 1 year ago

@nandorojo Sorry for the slow response. First off, I want to thank you for being so responsive and being such a great library maintainer alongside your job!

I tried that solution and it made 0 change to the performance metrics I shared.

nandorojo commented 1 year ago

as a sanity check, removing the skeleton altogether adds the time back?

ansh commented 1 year ago

Removing the Skeleton all together or keeping the count to 2 items make the stall_longest_time, stall_total_time, stall_count values all 0

nandorojo commented 1 year ago

Can you try the Reanimated v3 RC and see if that helps?

nandorojo commented 1 year ago

Alternatively, if you could create a reproduction where I can see these tests somehow, I can look into it. How did you set up Sentry to track this?

ansh commented 1 year ago

I tried v3-rc8 and rc9 and there were other bugs that I reported in the reanimated library. However, I can provide you with a reproducible demo so you can check it out. I'll set up Sentry with a free account and add you as a collaborator. Can you send me your latest email on Twitter? Just DM'd you.

nandorojo commented 1 year ago

Opening a PR here to potentially address this. Don't have much time to test it yet, so it's possible I messed something up since it's a full rewrite. But feel free to test it for now.

https://github.com/nandorojo/moti/pull/267

nandorojo commented 1 year ago

Has the new version of Moti helped at all by chance? Along with Reanimated 3.0 stable?