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

☂️ Identify Performance Issues #336

Open
2 of 3 tasks
nandorojo opened this issue Feb 18, 2024 · 16 comments
Open
2 of 3 tasks

☂️ Identify Performance Issues #336

nandorojo opened this issue Feb 18, 2024 · 16 comments

Comments

@nandorojo
Copy link
Owner

Is there an existing issue for this?

  • I have searched the existing issues

Do you want this issue prioritized?

  • Yes, I have sponsored
  • Not urgent

Current Behavior

Let's find if there are any performance problems that have been identified in #322 (and somewhere else for skeletons.)

I think there may have been a regression with Moti related to reanimated 3 causing issues for MotiPressable and Skeleton. It's possible it's a simple fix. I'd also like to compare Animated.View with MotiView directly to identify potential causes.

From what I've heard, the issue lies somewhere in useMotify. It is likely worklet related (I think).

I welcome any feedback and support on how to identify performance issues. Thank you!

Expected Behavior

It should be as fast as Reanimated raw.

Steps To Reproduce

No response

Versions

- Moti: Latest

Screenshots

No response

Reproduction

N/A

@nandorojo nandorojo changed the title Identify Performance Issues ☂️ Identify Performance Issues Feb 18, 2024
@fuelkoy
Copy link

fuelkoy commented May 29, 2024

After updating from Expo SDK 50 to SDK 51 (react-native-reanimated 3.6.2 to 3.10.1), there were big performance issues using Moti, but it seems to me that the problem lies within 'react-native-reanimated` as stated by @efstathiosntonas in #322. There have been multiple issues raised relating to performance degradation since version 3.6.1, in addition to 5800 namely 5816, 5781, even though all are not probably related to this one mentioned here in Moti.

I am just mentioning this here to add some information.

EDIT Let me come back to this after testing now. In my use case, actually changing to use pure react-native-animated animations instead of Moti fixed the performance issue. It was appearing when calling handleSubmit from react-hook-form with many inputs, and actually passing the handleSubmit took 2 seconds due to input rendering again as I had <InputHeader /> with placeholder text animations in it. So I changed those from Moti to react-native-reanimated, and now everything works as previously in Moti without performance issues.

I can show the code I used if requested. I don't have a repo to showcase this at the moment.

@TamasFarago
Copy link

After updating from Expo SDK 50 to SDK 51 (react-native-reanimated 3.6.2 to 3.10.1), there were big performance issues using Moti, but it seems to me that the problem lies within 'react-native-reanimated` as stated by @efstathiosntonas in #322. There have been multiple issues raised relating to performance degradation since version 3.6.1, in addition to 5800 namely 5816, 5781, even though all are not probably related to this one mentioned here in Moti.

I am just mentioning this here to add some information.

EDIT Let me come back to this after testing now. In my use case, actually changing to use pure react-native-animated animations instead of Moti fixed the performance issue. It was appearing when calling handleSubmit from react-hook-form with many inputs, and actually passing the handleSubmit took 2 seconds due to input rendering again as I had <InputHeader /> with placeholder text animations in it. So I changed those from Moti to react-native-reanimated, and now everything works as previously in Moti without performance issues.

I can show the code I used if requested. I don't have a repo to showcase this at the moment.

Can you show the code please? I also have the same issue

@Nadimkhan120
Copy link

i also faced performance issues on android using MotiPressable in lists, moti skeleton also very slow. mainly when using moti, there are always lag in animations like opening bottom sheet, navigating to a next screen, removing moti completely, my app is very smooth now. i was using latest moti with last reanimated.

@nandorojo
Copy link
Owner Author

Thank you for sharing! If anyone has a minimal repro it would help identify the issues a lot

@vlanemcev
Copy link

Hi! Thanks for this library!
Any plans to check performance issues in the near feature?

@nandorojo
Copy link
Owner Author

Hey I would really love to, I'm just slammed at the moment. Any help setting up an example in the repo showing a perf difference would help a lot

@nandorojo
Copy link
Owner Author

So far it's been pretty much a few anecdotes of someone mentioning an issue in a massive list so I want to narrow it down to an example if possible

@nandorojo
Copy link
Owner Author

Also want to see if it's related to Reanimated memory leaks: #322 (comment)

@fuelkoy
Copy link

fuelkoy commented Sep 3, 2024

@TamasFarago Getting back from holiday so I'll put some code here of the previous and current code. This is not a minimal repro but showing the use case where the issue appeared for me. This is pseudo code now giving minimal example:
(worth to note I'm using twrnc: tailwind-react-native-classnames, for styling that you will see in the InputHeader component)


function Input(props) {
	const [focused, setFocused] = useState(false); // To control focus state of the input (border color + animations for the placeholder)

// react-hook-form controller
const {
		field,
		fieldState: { error },
	} = useController({
		control: props.control,
		defaultValue: "",
		name: props.name as string,
	});

return (
<InputContainer
    focused={focused}
    inputValue={field.value}
    item={item}
>
<TextInput 
   {...item}
   // To change focused state
   onBlur={onBlur}
   onFocus={onFocus}
  value={field.value}
 />
</InputContainer>
)
}

function InpuntContainer(props) {
return (
  <View 
   // Some universal styling for inputs
   style={}
>
{props.children}
{text ? (
	<InputHeader item={props.item} focused={props.focused} inputValue={props.inputValue} />
) : null}
</View>
)
}

/**
 * Header for input.
 * @description Displays text, required mark, explanation (for web)
 interface 
 * @param {InputHeaderProps} props - The component props.
 */
export default function InputHeader({
	focused,
	inputValue,
	item,
}: InputHeaderProps) {
	const { animationDisabled, editable, info, required, text, webLogin } = item;

	/**
	 * Indicates whether the placeholder text should be animated up to the middle of the input top border.
	 */
	const shouldAnimatePlaceholder = useMemo(() => {
		if (
			(inputValue !== undefined && inputValue !== "" && inputValue !== null) ||
			inputValue === 0 ||
			focused
		) {
			return !animationDisabled;
		}
	}, [animationDisabled, focused, inputValue]);

	const phoneNumber = useMemo(() => item.name === "phoneNumber", []);

	const topPosition = useSharedValue(0);
	const topPositionAnimation = useAnimatedStyle(() => {
		return {
			top: withTiming(topPosition.value, {
				duration: 200,
				easing: Easing.inOut(Easing.quad),
			}),
		};
	}, []);

	const bgViewOpacity = useSharedValue(0);
	const bgViewOpacityAnimation = useAnimatedStyle(() => {
		return {
			opacity: withTiming(bgViewOpacity.value, {
				duration: 200,
				easing: Easing.inOut(Easing.quad),
			}),
		};
	}, []);

	const fontS = useSharedValue(web ? 12 : 15);
	const lineH = useSharedValue(17);
	const paddingLeft = useSharedValue(web ? 4 : 6);
	const fontAnimation = useAnimatedStyle(() => {
		return {
			fontSize: withTiming(fontS.value, {
				duration: 200,
				easing: Easing.inOut(Easing.quad),
			}),
			lineHeight: withTiming(lineH.value, {
				duration: 200,
				easing: Easing.inOut(Easing.quad),
			}),
			paddingLeft: withTiming(paddingLeft.value, {
				duration: 200,
				easing: Easing.inOut(Easing.quad),
			}),
		};
	}, []);

	useEffect(() => {
		if (!shouldAnimatePlaceholder) {
			if (web) {
				fontS.value = 12;
				lineH.value = 18;
			} else {
				fontS.value = 15;
				lineH.value = 17;
			}

			topPosition.value = 0;
			bgViewOpacity.value = 0;
			paddingLeft.value = web ? 4 : 6;
		} else {
			if (web) {
				if (info || webLogin) {
					topPosition.value = -17;
				} else {
					topPosition.value = -16;
				}
				fontS.value = 12;
				lineH.value = 18;
			} else {
				if (info) {
					topPosition.value = -13;
				} else {
					topPosition.value = -20;
					fontS.value = 14;
					lineH.value = 14;
				}
			}

			bgViewOpacity.value = 1;
			paddingLeft.value = web ? 10 : 14;
		}
	}, [shouldAnimatePlaceholder]);

	return (
		<Animated.View
			style={[
				{
					position: "absolute",
					left: 8,
					height: "100%",
					flexDirection: "row",
					alignItems: "center",
					justifyContent: "flex-start",
					overflow: "hidden",
					zIndex: -2,
				},
				topPositionAnimation,
			]}
		>
			<View
				style={tw.style(
					`flex-row rounded-3xl pr-0.5 h-[${
						shouldAnimatePlaceholder ? 14 : 18
					}px]`,
					animationDisabled && (inputValue || focused) && "hidden",
					editable === false && !shouldAnimatePlaceholder && "bg-transparent",
				)}
			>
				<Animated.View
					style={[
						{
							position: "absolute",
							width: "100%",
							height: 2,
							backgroundColor: "white",
							top: web ? 10 : 6.5,
							left: 2,
						},
						bgViewOpacityAnimation,
					]}
				/>
				<Animated.Text
					style={[
						{
							fontFamily: "Lato-Bold",
							paddingRight: 4,
							color: tw.color("neutral-700"),
							height: 20,
						},
						fontAnimation,
					]}
				>
					{text}
				</Animated.Text>
				{required ? (
					<BaseText
						text={"*"}
						style={tw.style(
							"web:leading-6 leading-4 text-themeColor",
							editable === false &&
								!shouldAnimatePlaceholder &&
								"bg-transparent",
						)}
					/>
				) : null}
			</View>
		</Animated.View>
	);
}


// Under here is the old component that used MotiViews and Texts. Some animation values have changed since commenting this out but the performance issue should show


// /**
//  * Calculates the top position for the placeholder animation.
//  *
//  * @return {number} The calculated top position.
//  */
// const topPosition = useMemo(() => {
// 	if (!shouldAnimatePlaceholder) {
// 		if (tw.prefixMatch("web")) {
// 			boxHeight.value = 0;
// 			return 0;
// 		}
// 		boxHeight.value = 2;
// 		return 2;
// 	}

// 	if (web) {
// 		if (tw.prefixMatch("web:lg") || info) {
// 			boxHeight.value = -17;
// 			return -17;
// 		}

// 		boxHeight.value = -24;
// 		return -24;
// 	}

// 	if (info) {
// 		boxHeight.value = -11;
// 		return -11;
// 	}

// 	boxHeight.value = -24;
// 	return -24;
// }, [shouldAnimatePlaceholder]);

// const fontSize = useMemo(() => {
// 	if (shouldAnimatePlaceholder) {
// 		if (web) {
// 			fontS.value = 12;
// 			return 12;
// 		}
// 		fontS.value = 14;
// 		return 14;
// 	}

// 	if (web) {
// 		fontS.value = 13;
// 		return 13;
// 	}
// 	fontS.value = 17;
// 	return 17;
// }, [shouldAnimatePlaceholder]);

// const lineHeight = useMemo(() => {
// 	if (web) {
// 		lineH.value = 18;
// 		return 18;
// 	}

// 	if (shouldAnimatePlaceholder) {
// 		lineH.value = 14;
// 		return 14;
// 	}

// 	lineH.value = 17;
// 	return 17;
// }, [shouldAnimatePlaceholder]);

// const test = useMemo(() => {
// 	if (shouldAnimatePlaceholder) {
// 		padding.value = 10;
// 		return 10;
// 	}

// 	lineH.value = 2;
// 	return 2;
// }, [shouldAnimatePlaceholder]);

// return (
// 	<MotiView
// animate={{ top: topPosition }}
// transition={{ type: "timing", duration: 200 }}
// style={tw.style(
// 	"absolute left-2 h-full flex-row items-center justify-start overflow-hidden",
// 	{ top: topPosition },
// 	info && "h-6",
// 	!web ? "left-3 max-w-input" : "xs:max-w-[86%]",
// 	phoneNumber && "left-26 web:left-21",
// 	{ zIndex: -2 },
// )}
// 	>
// 		<View
// 			style={tw.style(
// 				`flex-row rounded-3xl pr-0.5 h-[${
// 					shouldAnimatePlaceholder ? 14 : 18
// 				}px]`,
// 				animationDisabled && (inputValue || focused) && "hidden",
// 				editable === false && !shouldAnimatePlaceholder && "bg-transparent",
// 			)}
// 		>
// 			{/* <MotiView
// 				animate={{ opacity: shouldAnimatePlaceholder ? 1 : 0 }}
// 				style={tw.style(
// 					"absolute w-full h-[1.5px] bg-white top-[11px] web:top-[11px] left-0.5",
// 					webLogin && "top-2",
// 					{ opacity: shouldAnimatePlaceholder ? 1 : 0 },
// 				)}
// 			/> */}
// 			{/* <MotiText
// 				accessibilityLabel="input placeholder"
// 				animate={{
// 					// @ts-expect-error style property not recognized
// 					fontSize,
// 					paddingLeft: shouldAnimatePlaceholder ? 10 : 2,
// 					lineHeight,
// 					// fontFamily: test ? "Lato-Italic" : Lato-Regular,
// 				}}
// 				transition={{ type: "timing", duration: 200 }}
// 				style={tw.style(
// 					// { fontFamily: "Lato-Bold", fontSize, lineHeight, paddingLeft: 10 },
// 					// { fontFamily: "Lato-Black" },
// 					"pr-1 text-neutral-700 h-5",
// 					editable === false && !shouldAnimatePlaceholder && "bg-transparent",
// 				)}
// 			>
// 				{text}
// 			</MotiText> */}
// 			{required ? (
// 				<BaseText
// 					text={"*"}
// 					style={tw.style(
// 						"web:leading-6 leading-4 text-themeColor",
// 						editable === false &&
// 							!shouldAnimatePlaceholder &&
// 							"bg-transparent",
// 					)}
// 				/>
// 			) : null}
// 		</View>
// 	</MotiView>
// );

@faljabi
Copy link

faljabi commented Oct 3, 2024

Skeletons are really heavy and causing performance issues as well for me.

@faljabi
Copy link

faljabi commented Oct 3, 2024

So far it's been pretty much a few anecdotes of someone mentioning an issue in a massive list so I want to narrow it down to an example if possible

When navigating to a list with only 10+ skeletons is causing a little lag. If I place for example 50+ skeletons It hangs for couple of seconds before navigating to the list screen with skeletons.

@nandorojo
Copy link
Owner Author

@faljabi thanks for the update. Is this Android only? And are you using flashlist?

@faljabi
Copy link

faljabi commented Oct 3, 2024

For both iOS and Android. Android is even a bit slower.
Yes I am using shopify Flashlist

@jesusgarcia-lafise
Copy link

jesusgarcia-lafise commented Oct 7, 2024

hi, same issue here for v 0.29.0, I'm currently using it in some flatlists of my app

In some Android devices the app just got hang, it looks like a memory leak.

Had to disable the skeleton before publishing my app, we are trying to make a workaround.

For Android devices caused "ApplicationNotResponding: ANR" and for iOS devices caused "App Hanging: App hanging for at least 2000 ms"

@faljabi
Copy link

faljabi commented Oct 10, 2024

Will this be solved?

@kanmi-idris
Copy link

kanmi-idris commented Oct 15, 2024

Sighs, I just spent hours, trying to figure out why bottomSheet render was really slow. sighs

PS: This happens even when i manually set loading to false. so either it is actively loading or not, it still causes the component it is used in to have a slow render, especially when that render is triggered by a state update from an action

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

No branches or pull requests

8 participants