Discussion: performance and accuracy

[mbilenko-florio]

@jmeistrich Moving discussion into the new thread, beginning there #57

@mbilenko-florio Very interesting! I wasn't aware that useLayoutEffect fires so much earlier than onLayout... But it seems like it won't really help because updating the positions needs another render anyway?

I didn't completely understood how it worked https://reactnative.dev/blog/2024/10/23/the-new-architecture-is-here#uselayouteffect, you can check the example with tooltip there. From my understanding results of first render will be scrapped and never displayed to the user, if you modify state in the useLayoutEffect.

I think you're right that it could solve the blank first frame, but I don't think that's a major issue for most apps. Of course there are certainly some where it could be a huge improvement, like in the movies example where it's rendering lots of horizontal lists.

LegendList is working quite similar to @shopify/flash-list with disableAutoLayout={true} prop set. There are some cases where FlashList doing little bit more rerenders of the AutolayoutView component(it needs to pass current scrollOffset to the AutolayoutView(probably can be optimized). Most exciting thing about LegendList for me is it's lean core, absense of dependency on quite old https://github.com/Flipkart/recyclerlistview , which allows to create lots of interesting features. Because of this similarity, idea of AutolayoutView native module also can be reused between libraries.

I still think there's a lot of room for optimizations before we reach for native as the way to improve performance. I just spent today optimizing and #58 seems to have a pretty big impact! It has a lot of small changes, but the last one significantly reduces how often calculateItemsInView runs, and that seems to be a huge improvement (at least on my phone). I'm curious if you see good results from that?

I want to stress, that native module here is not really a performance optimization, it's only accuracy optimization. It doesn't really help to reduce numbers of re-renders, it just make sure not to display container at random positions(remove gaps and overlaps between containers).
I have a feeling, that little inaccuracy of LegendList in the placement of the containers can be it's superpower, because it can create effect of the less blanking, compared to FlashList, which tries to always order items one-after-another.
Performance of both FlashList and LegendList lay totally on shoulders of the JS recycler, no native module will make it faster.

As we talked about before, when switching between your "Implement advanced maintainVisibleContentPosition algorithm" commit and the one before it, I see it caused a drop in performance. But I was looking into it today and I can't figure out what specifically caused it. Especially since the reduction in number of calls to calculateItemsInView should reduce the overhead of the extra computations you added...

I will also try to find where lies performance issue with maintainVisibleContentPosition, because theoretically there should not be more calculations when maintainVisibleContentPosition is off.

[todorone]

It's not directly related, but i'll chime there if you don't mind. WDYT about adding optional fixedItemSize for fixed size lists? If it's defined, position of items could be calculated with a simple multiplication of index on item size and we may not pass onItemLayout function to items potentially improving performance. Fixed item lists are really common scenario so we can treat this case separately to make LegendList even faster.

[mbilenko-florio]

It's not directly related, but i'll chime there if you don't mind. WDYT about adding optional fixedItemSize for fixed size lists? If it's defined, position of items could be calculated with a simple multiplication of index on item size and we may not pass onItemLayout function to items potentially improving performance. Fixed item lists are really common scenario so we can treat this case separately to make LegendList even faster.

I thought about this, but not sure how much it will improve performance. If your estimated heights are equal to real height measured by the onLayout, it will just skip rerender.
We have one such fixed size item in the example app(Initial scroll index precise navigation), you can try commenting onLayout and see if there is any improved performance.

[todorone]

I thought about this, but not sure how much it will improve performance. If your estimated heights are equal to real height measured by the onLayout, it will just skip rerender. We have one such fixed size item in the example app(Initial scroll index precise navigation), you can try commenting onLayout and see if there is any improved performance.

Unfortunately, i can't quickly grasp all 1k+ loc of main LegendList and right now i don't have low Android device to proof it, but fixed items size is not just absence of rerenders from estimated to real position, it's:

• do not execute loops of size accumulations for every item when calculating container position, O(n) => O(1)
• onLayout function is not executed on UI thread
• do not call functions like getItemSize, getRowHeight etc and possibly other optimisations that i can't see

Yeah, i doubt it will improve performance significantly, but pretty sure it will be tangible on Android devices, although no proof so far...🤷

[mbilenko-florio]

@jmeistrich Ok, i have figured out, why LegendList sucked so much compared to FlashList(CPU usage) in the our example app. Apparently amount of work doing by ItemCard was quite different for LegendList and FlashList

Some of those hooks, presumably useViewability or useViewabilityAmount is quite CPU intensive, and in the end LegendList was doing much more work during the render.

Doing this change puts both of the libraries in the same conditions, with same CPU usage and FPS.

UPD: Suspect is

useRecyclingEffect?.(({ item, prevItem, index, prevIndex }) => {
        refSwipeable?.current?.close(); <--- this imperative handle
    }); 

It adds lots of additional work on the recycling of the element, and makes LegendList to behave much slower, can't easy fix it because useRecyclingEffect does not support dependency list.

[jmeistrich]

Some of those hooks, presumably useViewability or useViewabilityAmount is quite CPU intensive, and in the end LegendList was doing much more work during the render.

Ah that's a relief! I was worried there was a real performance problem. So in that case it would be just the reanimated function being slow. The purpose in that example was to reset the open state of the swipeable view. But it animates closed which is a bad solution anyway. I'm not experienced enough with reanimated / swipeable to know a better solution off the top of my head, but I'll play with that when I get a chance (unless someone else wants to solve it better).

For now I think we could just comment out all of those hooks for doing accurate performance comparisons. And maybe that should be a separate example?

[jmeistrich]

• do not execute loops of size accumulations for every item when calculating container position, O(n) => O(1)
• onLayout function is not executed on UI thread
• do not call functions like getItemSize, getRowHeight etc and possibly other optimisations that i can't see

I think onLayout itself is not taking much perf (but please prove me wrong).

I do think you're onto something about not needing to accumulate positions if every item is a known fixed size. Although especially after the most recent changes, that math isn't super expensive and isn't run super often. I think it could be a good optimization worth experimenting with (though it may already be fast enough that it's not noticeable) but we should punt it to after 1.0. I'll make the next post a todo list of potential optimizations that I'll update as we discuss ideas, just to keep track of everything.

[jmeistrich]

Before 1.0

(Empty)

After 1.0

• A fixedItemSize prop which could allow skipping a lot of the math and accumulating positions
• Solve the frame delay of repositioning after rendering
  • I have a theory that positioning with an Animated value might work. It would need to hide (or go offscreen) before changing the itemKey, then render with a new itemKey, then reposition. I've experimented and haven't gotten it to work well yet though.
  • Try doing an auto layout view in native code for more immediately accurate positions

[mbilenko-florio]

Some of those hooks, presumably useViewability or useViewabilityAmount is quite CPU intensive, and in the end LegendList was doing much more work during the render.

Ah that's a relief! I was worried there was a real performance problem. So in that case it would be just the reanimated function being slow. The purpose in that example was to reset the open state of the swipeable view. But it animates closed which is a bad solution anyway. I'm not experienced enough with reanimated / swipeable to know a better solution off the top of my head, but I'll play with that when I get a chance (unless someone else wants to solve it better).

I've create PR to fix that issue #62

For now I think we could just comment out all of those hooks for doing accurate performance comparisons. And maybe that should be a separate example?

Yeah, good idea, we need to have separate viewability example, to avoid bringing imbalance into benchmarks

[todorone]

I think onLayout itself is not taking much perf (but please prove me wrong).

I do think you're onto something about not needing to accumulate positions if every item is a known fixed size. Although especially after the most recent changes, that math isn't super expensive and isn't run super often. I think it could be a good optimization worth experimenting with (though it may already be fast enough that it's not noticeable) but we should punt it to after 1.0. I'll make the next post a todo list of potential optimizations that I'll update as we discuss ideas, just to keep track of everything.

Whatever you said totally makes sense, as all what i'm talking about is gut feelings(no real measurement, you *sshole).
I'm just super-excited that after those looooong years you were able to come up with JS-only fast list experience and i'm trying to think how to optimise everything possible(even minor things could matter for some Android devices, especially those like TVs i've been working with), so it definitely should be postponed after initial 1.0 release. Thanks again!

[mbilenko-florio]

One reason to create fixed element size mode is actually Movies example. Now each row is waiting to be displayed until onLayout of inner list is triggered, which is actually not needed, and we can display it much faster.

[jmeistrich]

True, and I think in real life that's what we would want. But the movie example is a bit contrived for slowness as a way to test performance. It's adapted from this comparison of FlatList vs. FlashList as a way to test against Flashlist: https://github.com/Almouro/rn-list-comparison-movies

But that's a great point and reminded me of something that I had forgotten. It used to feel much smoother because it would do a nested recycle - the horizontal list was keyed by index so the outer container recycling would recycle all of the child images. But as we improved accuracy and with the improvements to handling data changing, it seems like it's not doing that anymore, and the horizontal lists are fully re-rendering as their data changes. I wonder if there's something to that (edge) case and a way to optimize it...

[mbilenko-florio]

I think recycling is still happening, try to scroll one of the horizontal lists, you will see scrolled example over and over when you scroll forward, sometimes you will also see black items on the container that was already recycled - sign recycling of LegendList didn't went so well.

[jmeistrich]

I just did another round of testing and minor optimization - you were right that the Movies example was not slower for the reason I thought, but because it was delaying render until after layout, which isn't necessary in a fixed size list (as you mentioned in another thread).

And it was setting opacity to mask previous recycling bugs which are now fixed by your data changing improvements, so I removed that too.

And it was looking artificially worse than FlashList because the height estimations were wrong so I fixed that.

I added a waitForInitialLayout prop to encapsulate the rendering while invisible behavior, because it's not needed and just makes render slower in fixed size lists. So you may need to add that to your usage if you were depending on that.

And I added an onItemSizeChanged prop to catch item sizes being different than estimated.

[jmeistrich]

I've been playing around with trying to reduce the overhead of position updates and here's what I've found:

setNativeProps

I think the setNativeProps method is not great because it goes outside of React batching and is slower sometimes, and it didn’t improve things a ton. I had hoped it would reduce the overhead of adjusting a position but it seems to make it better in some cases and worse in others. So I think it's a no-go.

Animated

I also tried setting the position with Animated and Reanimated. That seemed to be faster but then it's running on a different thread so the position updates sometimes happen before or after the React commit, so even though it's faster it can cause gapping which looks worse. So I think it's also a no-go.

PositionContainer

Last I tried a new (old) idea. I moved consuming the position into a PositionContainer that listens only to position updates. That would mean position updates would skip the render overhead of the full Container, which may save some ms while scrolling quickly. I think that also means we could remove the need for the useMemo because that was mainly to protect against re-rendering items when position was changing.

useLayoutEffect

@michbil and I had talked about it before and I tested it again. It seems to fire up to 50ms faster (on iOS Simulator) than onLayout, and it looks like it may reduce gapping because it updates positions faster?

Conclusion

Those last two are in the useLayoutEffect branch. I'm not totally sure how much all of this helps because I'm on a plane and didn't get my slow phone out, so I'll test it more tomorrow. I tried to do the discreet ideas in individual commits so we can easily switch between them or cherry pick them into other branches. @michbil I tried the useLayoutEffect change on your quick-and-dirty-sort branch and it seemed like it improved quite a bit. I'm curious how it looks for you!

[jmeistrich]

I did a bunch more experimenting comparing performance before and after merging the “maintainVisibleContentPosition algorithm” PR #43 which seems to have regressed performance. It seems like a lot of it was mostly from changing the measure method to opacity. I had approved that at the time because I didn’t think it would affect anything, but it seems like it is a problem!

I think what’s happening is when you start scrolling it starts rendering containers which haven’t already laid out, so they display with opacity 0 at first. And then scrolling fast will pull even more containers out of the pool and keep doing that. So the gapping we’re seeing is because containers are rendering with 0 opacity. Then at some point all containers have laid out so performance seems fast again. At least I think that’s what it is…

I’m not sure why the offscreen method would be different though… I don’t quite know how things work in react native on mobile but I believe on web setting an opacity makes element rendering do more work involving the compositor to blend with the item behind, so maybe it’s something similar on mobile?

So I just made some changes to remove the individual containerDidLayout and have a single containersDidLayout. That appears to help a lot!

[michbil]

Single containersDidLayout is definitely way to go, i did it once, but it was lost in the numerous attempts and branches. It definitely caused some unneeded blanking, but it was only for the new containers, which are never was used. It was seen as short blanking blink during initial render, when number of containers stabilizes. I will do more testing, and report mu feeling about it.