ScrollView performs 10x slower if nodes are added.

[psionic12]

I found that if I add a node to a scrollview, the layout procedure is 10x slower. And I add logs to YGNodelayoutImpl and the printed result show that when scrollview's layout changes, YGNodelayoutImpl get called 10 times more than normal (50 times vs 500 times in my case).

After diving into yoga for a very long time, I found the root cause may because of layout caching missing. The interesting part is that when scrollview's layout changes, the shadow node do re-calculate the cache, but the cache is saved inside the particular node, and this node will be dropped when next rootCompileted is called, which means the calculated caches are also dropped, makes the cache calculate do again and again, which hurts a lot.

Is this a known issue? Anyone else noticed that scrollview hurts the performance? (I post here because I don't know whether this is a bug of yoga or a bug of RN).

[psionic12]

I wrote a JS demo in my fork, just use yarn install-android-jsc to build the rn-tester, and touch "Comment00" item before and after scrolling, and obvious the calling time of YGNodelayoutImpl (either by logs or using unsuspend break points).

[psionic12]

I just test the behavior when fabric is disabled, and I found that when I touch an item, the YGNodelayoutImpl are not get called at all, which is an expected behavior since I just change the background color, the layout is not changed. This makes the fabric layout more weird, when fabric is on, YGNodelayoutImpl not just get called, it gets called hundred times.

[psionic12]

My colleague wrote me a better demo in the branch I pasted, now, to illustrate the problem, press the "This is a very useful comment about the code." item, then press the "ADD COMMENT", then press the newly added "This is a new comment.".

According to my test, YGNodelayoutImpl get called about 30 times before the new item is added, and after that, when press the "This is a new comment.". YGNodelayoutImpl get called about 200 times.

[sammy-SC]

@psionic12 this is a great analysis, thank you! Btw, we just analysed performance of the New Architecture and published the findings #123. I think some of it may be relevant to what you observed here, especially the missing NDEBUG compiler flag, which makes layout considerably slower.

GNodelayoutImpl get called 10 times more than normal (50 times vs 500 times in my case)

This is expected. In Fabric, when ScrollView scrolls, it needs to update its content offset. This triggers Fabric's state update, not to be confused with React's useState hook. Fabric's state update triggers layout pass. In ScrollView's case it should be mostly just getting cached values. State of user interface is represented in shadow trees in its entirety, including content offset. This unlocks some interesting possibilities, for example asking how big a view is from JavaScript is no longer asynchronous call. Building on top of this, we can virtualise all views which are either outside of viewport or covered by other views.

I hope this clarifies why layout pass is triggered more often in Fabric.

After diving into yoga for a very long time, I found the root cause may because of layout caching missing. The interesting part is that when scrollview's layout changes, the shadow node do re-calculate the cache, but the cache is saved inside the particular node, and this node will be dropped when next rootCompileted is called, which means the calculated caches are also dropped, makes the cache calculate do again and again, which hurts a lot.

This sounds like a legit bug that is worth investigating. However, there is a special case: Text, or any shadow node with custom measure function. Their yoga node is dirtied every time they are cloned, forcing another measure pass. We use TextLayoutManager's cache here to make it fast. There are other options worth exploring. Cache is needed but Fabric could be smarted about how when it dirties yoga node.

[psionic12]

@sammy-SC Hi, I just have some interesting tests which seems to have relevant to your comments.
I disabled the reconciliation in ShadowTree.cpp to disallow the node to be marked as dirty, and whenever a new ShadowNode tree is created, I copied the layout cache from the old version, this causes layout problems, of course, but if a ScrollView doesn't change, the layout is very, very fast. I think copy the layout caches to the new tree maybe a solution, except that the reconciliation in ShadowNode.cpp needs to be smarter (currently if a ScrollView does not change, it will be marked as dirty).

[sammy-SC]

That indeed sounds like a room for improvement. Thank you!