Desktop: Performance: suppresses redundant SideBar re-rendering on state.tags #6451
Conversation
…tes of state.tags
| draft.tags = action.items; | ||
| if (!isDeepStrictEqual(original(draft.tags), action.items)) { | ||
| draft.tags = action.items; | ||
| } |
There was a problem hiding this comment.
It's strange that we'd need that because the point of immer is that it already performs this check for us. Otherwise we'd need to have everywhere if (value !== previousValue) draf.value = value but of course we don't (and if needed that, we may as well remove the immer dependency since it would no longer be useful).
There was a problem hiding this comment.
Not strange. Immer will find changes according to reference equality, but it doesn't check the deep equality of the found changes.
if (value !== previousValue) draf.value = value is not needed, because it checks reference equality which is performed by Immer.
There was a problem hiding this comment.
Right, that's correct, I was confused.
This reducer is also used in the mobile app and I don't think React Native has this "utils" module. We have "fastDeepEqual" which I think you can use instead.
By the way, I guess it should be easy to unit test this? You just do state = reducer(state, action); and compare the state before and after.
There was a problem hiding this comment.
This reducer is also used in the mobile app and I don't think React Native has this "utils" module.
Sorry, I wasn't aware of the point.
Finished. d1e7816
nodes.js/util/isDeepStrictEqual() -> fastDeepEqual()
|
There's conflict on the reducer file, please could you fix it? Once it's done we can merge |
|
Done. |
This PR is one of PRs resolving issue #6386 and improves UI responses.
Problem
When a notebook is changed using Sidebar, re-rendering of Sidebar happens three times as the Performance Analyzer view (A) in the below image.
In this case, data used for Sidebar don't change so many times, and the number of the times of re-rendering should be less.
Cause and Treatment
The cause of re-rendering is as follows.
state.tagschanges even if the content of the tag list does not change. Their semantic values are the same, but their object references differ.state.tags.To fix the problem,
state.tagsshould not be changed when a given new content of the tag list is the same.Result
After this PR is applied, as the view (B) in the above image, two Re-renderings of Sidebar are reduced from the above Performance Analyzer image. The total reduction time is 186 msec.