-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 implications with Carousel as an indirect child of ScrollView #247
Comments
Hi @pcooney10, Well, you are stepping into quite a nebulous world, to say the least ;-) Everything I've learned about As you've understood, I can't say for sure that rendering a You're right by the way: I've deliberately ignored my own advice in the example since it was featuring only a small set of items :-) One final thought: I honestly don't know if switching from a |
Hey @bd-arc, Thank you for the quick reply. I was able to put a quick bandaid on my app and increase performance by doing some of the pretty basic stuff. Namely:
I still plan to test nesting the My thoughts on why
To aid in my testing efforts, I have a question that you might be able to answer - does a component that's currently "Virtualized" fire the Thanks again for the great repo ! |
Hey @pcooney10, Thanks for summarizing the common steps everyone can take to improve their components' performance. I've been meaning to add something like this to the doc; if you're ok with that, I think I'll use your list :-) Regarding your question, I had the same one a while ago and tried to put it to the test. I remember testing it with two different versions of React Native. In the first one, the Note that I'm currently considering migrating to this component instead - a |
Of course ! You notes in the README.md were a very good starting point for me so feel free. The benefit of Outside of seeing if |
So far, the React doc was my reference on this matter. This means I've been using the Chrome Dev Tools, just like you. But I've just re-read the "Performance" part of the RN doc, and my conclusion is that I should definitely read it more often. The "Profiling" section contains a lot of insights and introduces very useful tools. I need to try this all! |
Hopefully will have some bandwidth over the next 2 weeks to dig into this more! Using Instruments for iOS and systrace for Android doesn't seem like it'll be fun, but I guess you gotta do what you gotta do. Also, I had implemented this suggestion ( |
I have 500 items and despite using the properties initialNumToRender and maxToRenderPerBatch (10 and 20 respectively) the carousel still renders all the items beforehand, which results in very slow performance. Not sure what I'm missing? |
@bd-arc apologies for slow response I was on hols. I had a look at the tricks but I either already comply or the trick would not solve the issue. Once again my key problem is that all 500 cards are materialised (rendered) upfront. Can you point me toward an example where the carousel is configured to render only a few items at a time? Thanks! |
@jarecsni Unfortunately, this clearly looks like a You may want to try this Snack example from #235; it shows that items are, indeed, not all rendered upfront (which is an issue in this use case). |
@bd-arc Thanks for the Snack example, I have checked it and made some interesting discoveries.
Do you want me to raise an issue with these details? |
Ok just seen an issue raised about the 2nd problem. However I think it only talks about swipes, whereas clicks events in general are missed. |
@jarecsni The root of the missing swipe/click events is the same: it has to do with the custom Regarding carousel performance, and as stated in prop
On a second thought, it would probably be a good idea to display this warning on the main page. |
OK thanks, that clarifies it. For me this means that I will need to go with default layout as the performance with deck is not good enough. Will keep an eye on the project for any fix you may do on this in the future. Thanks! |
It took me a long time to figure out that this was my root cause for the performance issues I see in my repo https://github.com/Norfeldt/LionFood_FrontEnd Say I drop the Is there any magic wand trick that you can recommend to me? - I don't have that many or deep decks of cards, but it's killing my app. |
Is this a bug report or a feature request?
General question
Have you read the guidelines regarding bug report?
Yes
Have you read the documentation in its entirety?
Yes
Have you made sure that your issue hasn't already been reported/solved?
Yes
Is the bug specific to iOS or Android? Or can it be reproduced on both platforms?
Both
Is the bug reproductible in a production environment (not a debug one)?
N/A
Have you been able to reproduce the bug in the provided example?
N/A
Environment
Environment:
React: 16.0.0
React native: 0.50
react-native-snap-carousel: 3.5.0
Target Platform:
Android (8.1)
iOS (11.2.2)
Steps to Reproduce
In the Performance Tips section of the docs there's a note that says:
Does this mean that
Carousel
shouldn't be a direct child ofScrollView
, or a child at all?In your example implementation, I noticed that you had
ScrollView
-->View
-->Carousel
, so I'm curious if this is conforming to the performance tip, or ignoring it since it's a just an example with a few items.As an aside, I'm really loving this component/library, so thanks for the great work ! I recently noticed some performance bottlenecks during the initial render, so I want to make sure I'm going about everything correctly. My current setup is a
ScrollView
with about 12Carousel
children (wrapped inView
's), that each have 5-20 items. I have a feeling items off the screen are getting rendered, so I might switch the parent component from aScrollView
to aSectionList
or aFlatList
instead.The text was updated successfully, but these errors were encountered: