VirtualThought: Re-rendering on typing

[raineorshine]

I noticed that typing got increasingly slow as I added many thoughts, so I debugged the rendering with React Dev Tools and Chrome Dev Tools. React Dev Tools pointed me to the biggest rendering bottleneck.

The problem is VirtualThought is rerendering unnecessarily, so for every keystroke, all the thoughts are being rerendered, which is very expensive. The reason is that even though it uses React.memo, memo performs a shallow comparison by default, and the path and simplePath props are arrays that I'm guessing are being cloned or recalculated, so the references change, provoking a rerender of LayoutTree and all of the thoughts every time (see screenshot 1).

The solution is adding a deep comparison to the React.memo function: const VirtualThoughtMemo = React.memo(VirtualThought, (prevProps, nextProps) => isEqual(prevProps, nextProps))

I tested this with the same 240 thoughts. The typing is smooth, and the rerendering is gone (screenshot 2).

The screenshots show that LayoutTree and its children's total rendering time decreased from 230.6ms to 2.5ms with this optimization.

I'm curious why path and simplePathare not stable refs. They shouldn't change when the user edits a thought. Could you investigate? isEqual fixes the problem, but it would be good for us to better understand the root cause.

My (naive) attempt to stabilize the refs before involved memoizing childPath:

https://github.com/cybersemics/em/blob/10b9d7c90faca1aec451bb104386269b4be53c11/src/components/LayoutTree.tsx#L273

...and using isEqual on the linearizeTree selector:

https://github.com/cybersemics/em/blob/10b9d7c90faca1aec451bb104386269b4be53c11/src/components/LayoutTree.tsx#L351

Off the top of my head I can't think of why the entire tree would be recalculated when the current thought changes. If there is a better way to do this I would love to find it.

[carloslfu]

I'm curious why path and simplePathare not stable refs. They shouldn't change when the user edits a thought. Could you investigate? isEqual fixes the problem, but it would be good for us to better understand the root cause.

I found that increasing the cache size of appendToPathMemo makes the issue go away until you hit enough thoughts to fill it again:

Regarding the selector: const treeThoughts = useSelector(linearizeTree, _.isEqual). I profiled it and did a few tests to understand how and why it provoked the re-renders. What is happening is that since a TreeThought includes a Thought, any change to a thought including lastUpdated and value will make the selector trigger a re-render of the LayoutTree component.

This could be solved by creating a custom equality function to replace _.isEqual, maybe using _.isEqualWith. This would be at the cost of added complexity, but it could positively impact performance when more thoughts are rendered. With my measures of the app with 200-300 thoughts, re-rendering the LayoutTree is not a problem as far as the VirtualThought's issue is fixed.

[raineorshine]

Regarding the selector: const treeThoughts = useSelector(linearizeTree, _.isEqual). I profiled it and did a few tests to understand how and why it provoked the re-renders. What is happening is that since a TreeThought includes a Thought, any change to a thought including lastUpdated and value will make the selector trigger a re-render of the LayoutTree component.

What if we changed TreeThought to store the ThoughtId instead of Thought? Then it wouldn't change during edits. The thought can be looked up in O(1) with getThoughtById(state, thoughtId) later on.

I found that increasing the cache size of appendToPathMemo makes the issue go away until you hit enough thoughts to fill it again:

Strange, I would think that the old cache entries would be invalidated and the newer paths would still get memoized. Regardless, maybe memoizing the path is not the best approach here.

I think the custom equality function is a good idea for linearizeTree. However, we need to be careful with simplePath and path. If they are unstable refs and they get passed as props, they will cause descendant components to re-render too often.

One idea is to encode/decode the paths. path.join(',') and pathEncoded.split(',') are very fast. Though somehow that seems wrong that we have to pass around serialized values in memory.

[carloslfu]

What if we changed TreeThought to store the ThoughtId instead of Thought? Then it wouldn't change during edits. The thought can be looked up in O(1) with getThoughtById(state, thoughtId) later on.

This is a good idea! I just tested this change, and the only place where thought properties from TreeThought are used in LayoutTree is here:

Therefore, sharing only the thoughtId is perfect!

Strange, I would think that the old cache entries would be invalidated and the newer paths would still get memoized. Regardless, maybe memoizing the path is not the best approach here.

It might be the combinations + the window. After 100 thoughts, and if you have nested thoughts, I can see how the refs could become unstable.

I think the custom equality function is a good idea for linearizeTree. However, we need to be careful with simplePath and path. If they are unstable refs and they get passed as props, they will cause descendant components to re-render too often.

I agree.

One idea is to encode/decode the paths. path.join(',') and pathEncoded.split(',') are very fast. Though somehow that seems wrong that we have to pass around serialized values in memory.

Yes! I thought about using plain strings too. I agree that it seems a bit odd, though.

[raineorshine]

One idea is to encode/decode the paths. path.join(',') and pathEncoded.split(',') are very fast. Though somehow that seems wrong that we have to pass around serialized values in memory.

Yes! I thought about using plain strings too. I agree that it seems a bit odd, though.

Probably the better approach is to use a custom arePropsEqual in React.memo. All components within the thought hierarchy are memoized, so we can do value equality on path and simplePath all the way down.

[carloslfu]

Yes, that sounds good.