brentvatne / hard-react-native-problems

I don't pay attention this, you may be more interested in https://github.com/lelandrichardson/react-native-future
164 stars 5 forks source link

ListView performance #3

Open devknoll opened 9 years ago

devknoll commented 9 years ago

Want to open a discussion, because I might be interested in contributing something here.

It seems like ListView should behave more like UITableView (I.e implement reusable cells?) - at least that's how native apps solve the problem.

Is react-native already reusing UIView hierarchies? ListView appears to already be rendering only what's visible (or am I mistaken?) so theoretically it should already be (or be able to) recycle a lot...

Would love some input from someone who knows more about it :) @brentvatne

devknoll commented 9 years ago

https://github.com/facebook/react-native/issues/499 Some previous discussions

devknoll commented 9 years ago

Downloaded the app to try it out. Seems like there aren't a lot of rows, so I must have misinterpreted the problem ;-) is it that switching between tabs is slow? That the tabs are being thrashed when switching?

brentvatne commented 9 years ago

Is react-native already reusing UIView hierarchies? ListView appears to already be rendering only what's visible (or am I mistaken?) so theoretically it should already be (or be able to) recycle a lot...

It is not reusing UIView hierarchies by default. You're right re being able to recycle a lot: https://twitter.com/jordwalke/status/627036346763186176

react's "key" can be used as a way to make react automate cell reuse - what you'd normally do by hand in UITableView

Check out https://github.com/facebook/react-native/issues/332 as well for further discussion

Downloaded the app to try it out. Seems like there aren't a lot of rows, so I must have misinterpreted the problem ;-) is it that switching between tabs is slow? That the tabs are being thrashed when switching?

Right, in the example I provided there are not a lot of rows - just a lot of tabs. So switching between these tabs becomes expensive - the reason for this depends on the implementation, in the example that I put together on my scrollable-tab-view repo it seems to be because I don't clean up after the views - each time you scroll to another tab, more and more are added to the view hierarchy :)

devknoll commented 9 years ago

react's "key" can be used as a way to make react automate cell reuse - what you'd normally do by hand in UITableView

Hmm, so something like maintaining a queue of keys in each render, reusing the old value for cells still in view, and recycling an old one for a new cell?

In facebook/react-native#499 the worry seems to be about state when reusing cells, but maybe with React's reconciliation this problem doesn't exist? (Or shouldn't?)

it seems to be because I don't clean up after the views - each time you scroll to another tab, more and more are added to the view hierarchy

Just for clarification, you just mean more are being created because there are so many tabs, not due to some garbage not being cleaned up, right?

devknoll commented 9 years ago

So, thinking about it a little more... key might be a tough way to solve the more general problem. It might do the trick in a single ListView (and would require some surgery), but not if you're scrolling between list views with the same type of nodes.

Here's a more generalized idea that I came up with:

  1. Update react-native to allow UIView tree recycling. When a request for a new component tree comes in, see if we've already got something with the same structure we can recycle.
  2. Somehow track each UIView to see if it's still needed. Two ideas:
    • Check periodically if the element is visible/within the bounds of the screen
    • Create a UIView subclass that tracks the last time drawRect was called
  3. Unmount nodes according to one of the above heuristics and let it be recycled somehow.

This could be opt-in or could be applied across the entire app automatically.

sghiassy commented 9 years ago

I wrestled with these questions a lot when creating react-native-sglistview. I REALLY wanted to implement cell reuse, but what I couldn't get my mind around was flex styling in a grid format.

So if style was something like: flexDirection: 'row', flexWrap: 'wrap', justifyContent: 'space-around'.

removing a cell would cause a reflow of the cells and they could potentially jump positions. That was a blocker I couldn't get around.

devknoll commented 9 years ago

Awesome, thanks for linking!

RE: Reflow of cells. Could you do both? Keep a lightweight <View width={} height={}/> like you're doing now, but reuse the userView? Maybe that wouldn't work for some reason though.

brentvatne commented 9 years ago

there is some more work being done here internally at fb, maybe we'll see something soon :)

robertjpayne commented 9 years ago

@brentvatne ooo can you share more? I've found it impossible to get a 60FPS scroller even on an iPhone 6S unless I leave removeClippedSubviews off which then explodes memory usage.

I came to the conclusion that the following must be implemented:

I couldn't find a way to easily get the reusing of views in react-native unfortunately.

rreusser commented 9 years ago

@robertjpayne FWIW and probably not relevant, but we were struggling with smooth scrolling. I thought the problem was too many pieces in a component. Or lack of cell-reuse. Or a failure to correctly use removeClippedSubviews. Or that SGListView was rendering too many elements. Turns out though that the problem was a large shadow on an element. It appears that shouldRasterizeIOS has fixed our problem with basically one line of code.

Your situation may be totally different, but ours was that a single problem-item was killing our rendering performance. I spent time investigating these other issues and trying to squeeze more performance out of the listview. I made some progress, but it appears to have mattered way less than locating and fixing the actual problem.

Just a thought!

rreusser commented 9 years ago

(And not, of course, to imply that ListView doesn't remain a hard and relevant problem!)

aleclarson commented 8 years ago

there is some more work being done here internally at fb, maybe we'll see something soon :)

I'd love to hear more about the work / ideas! :grin:

What @robertjpayne brings up sounds promising.

brentvatne commented 8 years ago

A good discussion that is related to this: https://www.facebook.com/notes/andy-street/react-native-scheduling/10153916310914590

rclai commented 8 years ago

Just wanted to follow this discussion as well as share what I faced.

I was trying to implement this kind of scrolling behavior using a ListView but the paging in of new rows would completely destroy the performance. I was tracking the scrollY and syncing it with an Animated.Value and interpolating the sub-view's scale based on the scrollY.

sibelius commented 7 years ago

WindowedListView on RN core works fine

el-lsan commented 7 years ago

@sibelius could you please leave me an example on how you handled the data prop ?

I keep getting following error, while my array seems to be fine: image

my data prob image

I'm using RN 0.38.0

sibelius commented 7 years ago

data is a list of object that has 2 keys

{
   rowKey: 'unique key'
   rowData: {id: 'id', name: 'name', ...}
}
el-lsan commented 7 years ago

@sibelius Thanks for your reply.

I just found out that the issue was not with my data prop but with importing. For some weird reason it works if I import the WindowedListView this way import WindowedListView from 'react-native/Libraries/Experimental/WindowedListView.js';

but this would fail 😕

import {
  WindowedListView
} from 'react-native';
sibelius commented 7 years ago

yep, that's true.

this is because this is an experimental component

it lags a bit on android https://github.com/facebook/react-native/issues/11950