-
Notifications
You must be signed in to change notification settings - Fork 839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More strategic useMemo
usage to improve code readability and performance
#77
Comments
I'm curious if you have quantitative data about the overhead in this particular case. Could you add that if you do? |
@charkour That is a very valid question. Unfortunately the official React documentation doesn't go into more detail than suggesting it be used for "expensive" calculations. However, I did find an attempt at quantifying the performance. The main takeaways from that article is:
The real issue that I see with premature optimization is the poor developer experience resulting from it. The potential gain in processing time is not significant enough to offset the cost in developer time. That time can later be used to optimize where it truly is necessary. |
Roger, thank you for the write-up! Looks like the maintainers aren't super active here, but I appreciate you raising concerns! |
There is currently a fair bit of unnecessary
useMemo
use in the codebase. It creates a cluttery codebase that may be off-putting to would-be contributors while not actually leading to improved performance. It will even hurt performance due to the overhead of the hook in many cases.Here is an example of such a case:
StableStudio/packages/stablestudio-ui/src/Generation/Image/index.tsx
Lines 153 to 165 in 76772d7
In this case, the overhead of
useMemo
is going to be significantly worse than letting React discover on its own that it doesn't need to rerender the component.useMemo
should ideally be used sparingly and for truly resource-intensive tasks, or in some special cases such as ensuring referential equality.It is also (almost always) a bad idea to wrap the return statements of functional components in
useMemo
as React is VERY good at knowing when it needs rerender components.I really think it would be good to clean this up if it is okay with the product owner(s). Doing so would lead to a friendlier codebase that may attract more contributors and would likely make StableStudio more optimized in the process.
The text was updated successfully, but these errors were encountered: