-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Disconnect from redux #2088
Comments
This sounds like you're asking for the still WIP React |
@markerikson any idea how to implement it using current api? App is very slow already, no time to wait couple more years to have this feature released and all libs updated. |
Afraid not, no. Never worked with React Native, never used How many components and how many active store subscriptions are you dealing with, that this is becoming a meaningful perf issue? |
afaik |
@markerikson it is an issue for web apps too, but making slow web apps looks like a standard in web development right now. Do you know any web app that is as fast as, for example, nice iOS app? On a slow device with low battery. And it is possible, if web devs and libs weren't so bad and companies spent some time on profiling and optimization. But lags and freezes on mobile apps is not something that users are used to, 60 fps is a standard, so the expectations are much higher. As for subscriptions/selectors, I'm not sure whats the best way to get the number, but I think there are hundreds of them. There are 5 screens in tab navigation, each can have stack with other screens. Each screen is a list of components, each of them is subscribed to redux. Also, there are subscriptions in root components like App (authorization, etc). And each action (including saga) causes to all of them recalculate. There can be dozens of actions each second. |
@gentlee this is getting a bit off the original question, but: what are you doing in those sagas that requires dispatching "dozens of actions a second"? That sounds excessive, and that starts to get into territory where Redux isn't necessarily well suited anyway. fwiw I do care very deeply about React-Redux perf, and I have some research avenues I'd like to investigate like #2033 / #2047 , but that'll be well down the road. my first general suggestion would be to look at the app architecture and see if you can cut down on the number of actions being dispatched in these cases. |
@markerikson there is an opensource app built on react native called mattermost. With redux it was extremely slow (1st version which is probably still in their github). So they swtiched to watermelonDB in the 2nd version (I don't think it was a good solution). This app could get up to 100 actions per second and even more, because it is a chat app with websocket. I've optimized it a lot in a fork once, batching updates to redux, and reduced it to around 10-20 actions per second. The app was still slow on android because each mounted component had a subscription to redux too (theme was also stored in a single redux store 🤦♂️), each emoji of each message had a subscription to redux. So dozens of actions is not a big number actually. Also, on app start there could be dozens of requests sent to the backend and it is fine - each mounted screen of tab bar can send its own, and then get response, all initializations of all services - geolocation, in-app etc. PS. They used |
@gentlee if you can record some kind of JS perf trace capture (equivalent to a Chrome DevTools perf trace JSON format), I can try to take a look and get a sense of how much time is being spent on subscriptions vs re-rendering vs other issues. The cost of individual subscriptions is not high. Several hundred subscriptions does have a cost, as every function call has a non-zero cost, but the larger question is really about the number of actions being dispatched, how long the selectors take to run, and how many components are being forced to re-render as a result. |
@markerikson I got one.
Could be watched with |
@gentlee Okay, loaded that into https://perfetto.dev and did a quick look for Looking at this screenshot for the first hit: Your Similarly, a later hit here: took 91ms to run. That's hideous :) That should never be happening. Selectors are intended to be very fast, for exactly this reason. A given selector should only take a fraction of a millisecond to run every time - it should either be doing a simple return of Looking at those stack traces, it seems like the real issue is the date handling. That looks like a lot of generating some kind of date config and parsing ISO strings, or something along those lines. Fix that issue first so that these selectors don't keep taking that long, and your app perf will improve dramatically :) |
@markerikson Yes I'm currently optimizing mapping functions, but there are too many calls anyway. Also, there are a lot of re-renders and garbage collection as well, caused by useSelector re-evaluations (that could be disabled). And yes, regular expressions (from using moment.js) are very slow, that are calculated in mapStateToProps functions. |
@gentlee : if a selector is properly memoized, it shouldn't be generating re-renders or garbage collections, because it will end up as a no-op, not return a new reference, and not force a re-render. the regex issue is an app-level concern. the fact that these are in every one of those calls tells me the selectors aren't properly memoized in the first place, otherwise they'd be avoiding the extra calculation attempts and be skipping the regex calls. so, yes, my advice here is that your app looks like it would first benefit from a major overhaul of the selector usage, and that would significantly improve the app perf, separate from the question of how many |
@markerikson lets say we have 300 selectors which evaluate for 50ms. It is 15 seconds freeze. I will optimize them to be 5ms on old android device, it will be 1.5 seconds of unresponsiveness, which is also terrible. But it could be 50 selectors of 5 ms - only 0.25 seconds. So number of active selectors also matters. Also, when you open details screen you often update data from the list screen, and this data can be used by other mounted screens. So memoization doesn't always help. |
To sum up: Current implementation has O(n) complexity, where n is number of selectors (of mounted components), while I'm searching for a O(1) solution. |
@gentlee : I agree that fewer calls to selectors will be less expensive that more calls. No argument there. But, React-Redux will never be an "O(1)" approach. It's O(n), by definition. My point is based on these perf profiles, your selectors are very badly written right now. That is the immediate problem. Fix those selectors, and the app perf will greatly improve, and it's likely that it will improve enough that the cost of components subscribed in the background won't be an issue at that point. |
If you properly memoize your selectors it will for an action be 295 that take <0.01ms each and 5 that maybe take 3ms. |
@markerikson it could have something like O(1) if I could disable all selectors of screens except currently visible one. Then even this very bad selector would be fine enough. Again, I'm already optimizing these selectors (this cpuprofile was done before any optimizations), but from my experience it is not the only thing that should be done. You can check actually mattermost, they have very good memoized selectors, but the app is extremely slow (version 1). |
And we're not talking fast or slow device here. A sufficiently memoized selector will be <0.1 on a potato with energy savings mode. |
Honestly: it's a problem of See https://github.com/software-mansion/react-native-screens#experimental-support-for-react-freeze import { enableFreeze } from 'react-native-screens';
enableFreeze(true); |
@phryneas they mount screens to switch tab bar faster. Also, you can swipe back with your finger on iOS and the screen below should be mounted not to have a lag. Native frameworks do the same, for example UITabBarController keeps all views "mounted" all the time, same as UINavigationController. You just unsubscribe them from updates when they disappear and subscribe when appear. But react-redux lack that ability. Yep, I'll check that freezing as well, thanks for idea. |
What is the new or updated feature that you are suggesting?
There should be a way of disconnecting from redux, which prevents both re-calculation of selector result and re-rendering of the component.
Why should this feature be included?
The main issue of bad performance of apps that use
react-navigation
or its analog with keeping components mounted when you switch them (like tab navigation or stack navigation) is subscription to redux. Especially relevant for React Native apps (Any app with tab bar and/or navigation stack).Similar issue was created for
mapStateToProps
once upon a time.What docs changes are needed to explain this?
There should be a new option in useSelector, smth like:
Can be used like:
The text was updated successfully, but these errors were encountered: