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

Performance issues #570

Closed
brunolemos opened this issue Feb 24, 2019 · 11 comments
Closed

Performance issues #570

brunolemos opened this issue Feb 24, 2019 · 11 comments

Comments

@brunolemos
Copy link
Contributor

brunolemos commented Feb 24, 2019

The code is this one, two useTransition at the same screen.

Works ok on iPhone X.
I notice some lags on Web sometimes and on Android 100% of the time. @Wmitrut reported this:

kapture 2019-02-24 at 19 04 23

Not sure if it's a regression from 7x -> 8x or we just really need #299.

@drcmda
Copy link
Member

drcmda commented Feb 24, 2019

imo that looks too slow to be just a fps issue, but needs to be profiled in order to look into what's causing it to stall like that. is it possible to extract this small piece with mock code around it into a codesandbox?

@brunolemos
Copy link
Contributor Author

I don't think it's anything specific to this code, any transition should be enough to debug.
Just need a mid-end device.

@brunolemos
Copy link
Contributor Author

brunolemos commented Feb 24, 2019

Ok, I made a quick investigation and there's definitely room for optimization.

There are too much script being run while the animation is running.
For example, the callback of injectCreateAnimatedStyle is being called more than 1000+ times on my app and this method (that contains StyleSheet.flatten) is a bit expensive.

And it seems to be called multiple times while the animation is running instead of only once, see:

kapture 2019-02-24 at 20 34 36

image

@drcmda
Copy link
Member

drcmda commented Feb 24, 2019

that could make sense, flatten can be memoized away. actually, it used to be like that, but when createAnimatedComponent was converted to hooks the complex shouldComponentUpdate stuff that was in there was moved out. i asked vincent riemer about this and we both agreed that it wouldn't cause any trouble. i still find it hard to think that this is a bottleneck - it's not intended to be run every frame.

@drcmda
Copy link
Member

drcmda commented Feb 24, 2019

what's weird weird about this, ... why is createAnimatedComponent.attach (https://github.com/react-spring/react-spring/blob/master/src/animated/createAnimatedComponent.tsx#L83) called so often? This is the one that creates animatedProps/styles, which should be one time. But why is the component re-rendering so often?

could you put a log in there and confirm that it's being called often? If that's the case we have at least found the cause b/c that's not normal.

@brunolemos
Copy link
Contributor Author

brunolemos commented Feb 24, 2019

hum...

It seems one of the main causes of this is because I have too many animated components.
Because the app supports multiple themes, basically all texts and views with background are an animated component. I don't think react-spring was made with this case in mind, so maybe it's not optimized for this yet (but maybe it could?). I did this way because theme change was causing a re-render of the whole app before, which used to take a couple seconds.

So, looking at my Settings screen, I have ~30 "animated" components. The console.log shows ~170 occurrences of the call when opening/closing the modal.

@brunolemos
Copy link
Contributor Author

I added a console.log below attachProps(props) and same thing happened.

@brunolemos brunolemos changed the title Performance issue with useTransition Performance issues Mar 27, 2019
@aleclarson
Copy link
Contributor

Could you test v9 on this? yarn add react-spring@next

@brunolemos
Copy link
Contributor Author

brunolemos commented Apr 23, 2019

@aleclarson didn't notice any perf improvement unfortunately. Android is still super slow or not working.

did notice a useTransition regression #642 (comment)

@brunolemos
Copy link
Contributor Author

brunolemos commented Apr 26, 2019

I was using react-spring on hundreds of components to avoid triggering a rerender on app theme switch. I changed this and now I only use on a few components that actually have animations.

Performance improved considerably after this change, but there's still a long way to go on android.

But the question is: why having react spring on lots of components causes lag even if they are not being animated or updated at all? something might be off on react-spring internal code.

Possibly related: #654

Also I think we really need #299

@brunolemos
Copy link
Contributor Author

brunolemos commented Jun 9, 2019

I FINALLY found out what was causing these lags. And it was a react-native bug, not a react-spring one. Here's the details: facebook/react-native#25201

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

3 participants