Skip to content
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

RTK with Immer x100 slower than vanilla Redux at average (up to x400) #4793

Open
gentlee opened this issue Dec 28, 2024 · 9 comments
Open

RTK with Immer x100 slower than vanilla Redux at average (up to x400) #4793

gentlee opened this issue Dec 28, 2024 · 9 comments

Comments

@gentlee
Copy link

gentlee commented Dec 28, 2024

I've wrote some benchmarks trying to understand how Immer influences the redux performance and got some very strange results:

  • the worst case is RTK slower than vanilla Redux by 408 times, and at average it is slower by 100 times...
  • even using immutable approach with original doesn't make much difference.

There are probably some mistakes in this code (initially generated by AI), so appreciate some help to find that out.

Here is the code.

To run: NODE_ENV=production node --expose-gc benchmark.mjs

Environment:

  • Node v22.12.0
  • Mac M1 16Gb

Results:

Data, ms (the lower the better)
  Vanilla Immer Immer NoAutoFreeze Immer Immutable Immer Immutable NoAutoFreeze
Add Item 131.3 13450.59 11812.4 11857.97 26037.62
Remove Item 358.88 24837.13 23621.49 8038.03 7194.04
Update Item 423.92 15680.84 14181.17 9658.24 28933.6
Concat Array 120.51 49178.25 47117.22 10940.38 36005.91
[Average] 258.65 25786.7 24183.07 10123.66 24542.79
Data, x times slower (the lower the better)
  Vanilla Immer Immer NoAutoFreeze Immer Immutable Immer Immutable NoAutoFreeze
Add Item 1 102.44 89.96 90.31 198.31
Remove Item 1 69.21 65.82 22.4 20.05
Update Item 1 36.99 33.45 22.78 68.25
Concat Array 1 408.08 390.98 90.78 298.78
[Average] 1 99.7 93.5 39.14 94.89
Chart, x times slower

Image

PS.

  • Initially test array had 100,000 size, but decreased to 10,000 to be closer to the average apps. Results are for the second.
  • Immer means using RTK's configureStore and createSlice. Vanilla means using createStore from redux.
@phryneas
Copy link
Member

phryneas commented Dec 28, 2024

Hmm. This is admittedly slower than I would expect - I would expect an "add item" immer reducer here to take more around 2ms than 6ms. This might be worth looking into.

That said, the 100x/400x from your headline here are highly artificial and hopefully never occur in a real application, and that also goes into the whole benchmark a bit:

See the style guide in these points:

The idea here is that an action should represent an event happening in your application - in almost all situations, those are either user-induced events (clicks), or things like the start or return of a network request.
That kind of action happens usually less than once per second, although it might sometimes "cluster" to up to 10 times per second (e.g. if on application start a lot of inpendent network requests are made). I'd never expect it to happen more often than that in a typical application though.
In a typical application, the render coming after a store update would be more expensive than the store update, so you'd try to prevent too many actions in the first place.
In those environments, you might see one action that adds 5000 elements, but you wouldn't see 5000 actions that add one element each.

Of course there are exceptions to this - what comes to mind are cases where we've seen Chatbots processing messsages from hundreds of users, or financial dashboards showing real-time data coming in via websocket. These cases can be optimized, or in a single reducer you might actually choose to opt out of immer by just writing a reducer by hand - but these cases make up the 1% of poweruser edge cases, not the 90-95% of users we primarily target and for whom we want to avoid actual real-life bugs.
In these poweruser edge cases we expect that people take closer care, maybe follow immer's performance tips, opt out by writing a reducer by hand, or frankly: not use Redux. If you need the pinnacle of performance, even a plain reducer might be doing too much work, since you always have to immutable clone everything.
This might just be a use case where you would want to start mutating objects and sidestep a state manager.

That said - I already started with this: these numbers feel too high by a factor of 3-5 to what I'd expect, and it is worth investigating.

Could you maybe adjust the benchmarks to resemble real-life application usage a bit more?
Also, some remarks regarding the benchmark:

  • please use createStore or configureStore in all benchmarks - it doesn't really make sense to run some benchmarks through additional middleware, while omitting that for others
  • the output is a bit confusing - could you change it to console.log(${name}: ${((end - start)/runCount).toFixed(2)});?

@gentlee
Copy link
Author

gentlee commented Dec 29, 2024

About actions per second - I was once participated in a project that had around 100 actions per second - it was a fork of opensource mobile chat app Mattermost, they recently switched to watermelon db (bad solution). Also many projects got lots of actions per second on app launch when fetching lots of stuff.

Also, we need to keep in mind that there are mobile devices, and even more, android mobile devices (x10 slower than iOS), and even more old android mobile devices with low battery, so for a good app there should be a pretty big performance reserve for such cases, if comparing to desktop.

And actually redux does pretty well in terms of performance. I would say much better than I expected. So it has a great potential in performance, but looks like not with immer. It adds a huge limitations here, and that chatting app would definitely freeze out if someone decided to use it (they used vanilla redux back then), as it already had performance issues due to some very bad decisions like keeping theme in single redux store and many more.

So not using redux when it actually could work very well is not cool honestly. There probably are some situations where there can be better solutions, but it is definitely not a chatting app.

  • Adjusting the benchmarks to resemble real-life application: I think 10000 length for an array is a good starting point, 1000 would be too small. I would consider a good chatting app with infinite pagination as a default target. But currently don't have time to improve it further, may be later.
  • createStore removed. It showed pretty much the same performance as configureStore.
  • the output changed.

Also did some refactoring etc.

EDIT: As for making plain reducers - they don't have all this cool boilerplate of slices, so not sure why not just disable immer for them / for slice reducers.

Also, I guess there definitely should be a tip in performance doc about that.

@markerikson
Copy link
Collaborator

markerikson commented Dec 29, 2024

I don't understand what you're saying with this sentence:

As for making plain reducers - they don't have all this cool boilerplate of slices, so not sure why not just disable Immer for them / for slice reducers.

As Lenz said: Redux was never intended to be the fastest lib out there. The primary goal is predictability, and accidental mutations go against that.

Sure, I wish Immer was faster :) I can see that the majority of the time in your benchmark is being spent in Immer's finalizeProperty internal method:

Image

I know @mweststrate has spent a lot of time trying to optimize it, but also have it be correct in a variety of update situations. Maybe there's still something that could be done to speed it up, maybe there isn't. Might be worth filing a perf issue over in the Immer repo to discuss this.

So yeah, in cases where you're using Redux and perf is still critical, you may feel the need to hand-write those reducers.

But overall we still see Immer as the right default choice for RTK.

@gentlee
Copy link
Author

gentlee commented Dec 29, 2024

I don't understand what you're saying with this sentence:
As for making plain reducers - they don't have all this cool boilerplate of slices, so not sure why not just disable Immer for them / for slice reducers.

  1. I meant that when someone needs original good performance of vanilla redux it would be nice to be able to use createSlice (or createPlainSlice, whatever) with disabled immer to get all that sugar with generated actions, selectors etc without the need of using plain reducers and actions with all their boilerplate.

  2. But even more perfect would be to make it possible to disable it by default for the whole RTK and remove that dependency, make RTK more modular - an option for experienced devs. Though I'm sure it won't be done in the near future.

And I even agree that enabling immer by default is probably fine. At least removing it by default on current stage can be very confusing.

@EskiMojo14
Copy link
Collaborator

Immer has been something we've been unwilling to budge on before (#242) and I don't see that changing soon.

We have an open thread/PR for investigating supporting immer alternatives such as mutative (which may perform better), but it's generally been pretty low priority.

@gentlee gentlee changed the title RTK x100 slower than vanilla Redux at average (up to x400) RTK with Immer x100 slower than vanilla Redux at average (up to x400) Dec 29, 2024
@phryneas
Copy link
Member

I meant that when someone needs original good performance of vanilla redux it would be nice to be able to use createSlice (or createPlainSlice, whatever) with disabled immer to get all that sugar with generated actions, selectors etc without the need of using plain reducers and actions with all their boilerplate.

The problem with that is that you get something that looks the same, but actually requires you to write completely different code, or you'll end up with a bug-ridden mess. Honestly, I don't want to enourage that, especially in the age of LLMs that just see "looks similar" and then spit out code they've seen a million times.

I'm fine with exploring alternatives to immer, but I am very hesitant on going without an immutability helper. Code that needs to be written in immutable style should look significantly different than code that can be mutable.

an option for experienced devs

Even for an experienced dev, this is a mental hurdle - switching between projects where one requires immutable code and the other one doesn't, it's easy to accidentally use the wrong paradigm if it's not obvious what is required. Neither an option nor a method name helps here - the case reducer object can easily become mutliple screens in size, so you won't be aware what method name you used, or what option you set somewhere.
And that's assuming that everone on the team is an experienced dev, which oftentimes isn't the case. You'd need a perferctly aligned team, perfect onboarding, extremely thorough reviews, etc. etc.
That might exist, but it's not very common.

Honestly, in those cases I'd very much prefer if people used createAction and hand-write a reducer.

@gentlee
Copy link
Author

gentlee commented Dec 29, 2024

  1. I agree that writing in completely different styles is something that should be avoided if possible. And actually this problem was not presented for many years when there was no immer. Yes, most of the time when devs asked me why something is not working I was looking at reducer first, and they were mutating state. But this happened only on the initial stage of development and only for inexperienced devs - subscriptions just don't work when you mutate state, so you are forced to fix that to make feature work and UI udpate. Also, this was a problem on every interview that I asked to fix during online coding, and most of devs located and fixed such issues pretty fast.

And yes, sometimes immutable code really looks ugly when it is complex, while it is pretty simple for simple cases.

When immer appeared, a good alternative was using immer for complex cases, while keeping immutable approach as basic. And I believe everyone who uses redux must know the basic concept and must be able to write immutable code.

  1. Perfect alternative to immer in my opinion would be something like a babel plugin that transforms mutable code to immutable at compile time, so that there is zero overhead. 100x slow at runtime is unacceptable.

That being said, I strongly believe that:

  • every dev who works with immutable state today should know both approaches, and understand their limitations.
  • there should be a way to choose default approach for the app. If it Is closer to hello-world - you can add immer or other lib as peer deep and enable by default. If it is a chatting app you need to be able to not enable it by default and even never use it if you don't want. So when someone onboards on a project, after seeing redux as dep he should ask what mutation approach is used for reducers - and this is fine. And approach should not change the RTK API that you use - you should still be able to use a very nice createSlice.
  • consider creating something like babel plugin to make it the perfect way possible, with zero overhead.
  • consider making performant immutable utils (if not yet exist) to make the immutable code simpler, and use them with immutable approach. For example in JS there is no way to clone Set while adding or removing element. There can be cloneAndPush, cloneAndUnshift array methods etc that return array for nice chaining calls.

EDIT: This problem may also significantly decrease the popularity of redux. Devs often like performance even more than it really matters - and you can't do anything about it. It is a risk that no-one wants to have in their project.

@phryneas
Copy link
Member

every dev who works with immutable state today should know both approaches, and understand their limitations.

I wholeheartedly disagree here - it's a massive bonus that we don't force people to learn both of these approaches. They can learn them once they need them.

there should be a way to choose default approach for the app. [...] - you should still be able to use a very nice createSlice.

createSlice is very few code (<80 lines with very generous formatting for everything) - it's easily possible to copy-paste the runtime code into your app and use a variant of it without immer in your code base without us shipping it - without us having to ship a footgun.

consider creating something like babel plugin to make it the perfect way possible, with zero overhead.

This is an Open Source project run on the weekends by 2-3 volunteer maintainers. Experiments like this are welcome, but they can also exist outside of RTK - I don't think we have the bandwidth to implement or maintain something like that.

consider making performant immutable utils (if not yet exist) to make the immutable code simpler, and use them with immutable approach. For example in JS there is no way to clone Set while adding or removing element. There can be cloneAndPush, cloneAndUnshift array methods etc that return array for nice chaining calls.

Again, this is outside the scope of what we can do. This could easily live outside of RTK and be it's own thing.

This problem may also significantly decrease the popularity of redux. Devs often like performance even more than it really matters - and you can't do anything about it. It is a risk that no-one wants to have in their project.

You are dismissing one important point here:

Redux Toolkit is the recommended default way of writing Redux since 2019. That's over 5 years, and more of half the time Redux exists. In that timeframe, RTK had about 430 million downloads.
This performance drop was a problem for half a dozen people that reported back in that whole time.
It doesn't present itself to most users.

This did not significantly reduce the popularity of Redux - since in most cases, it doesn't present itself at all.
The average application has a few dozen slices and they do not hold frequently changing arrays of arrays with megabytes of data, but oftentimes shorter arrays and objects.

Yes, this is something that needs investigating, but please stop overplaying this like "RTK is 400x slower for everyone". Your use case is an extreme outlier, and for most people the slowdown is insignificant.

We are in the unique situation where we have to strike a balance between "perfect performance for everyone, but dangerous footguns" and "perfectly viable performance for 90-95% of use cases, but no footguns".

I do not want to introduce an option that adds a significant speed improvement for a dozen apps while making it easier to introduce bugs in tens of thousands of apps.

@markerikson
Copy link
Collaborator

Yeah. To be clear:

We do care about performance. We want Redux, RTK, and React-Redux to be as fast as reasonably possible. I personally have put in a lot of effort over the years working on perf optimizations. Upgrading to Immer 10 to get its perf improvements was one of the big goals in getting RTK 2.0 out the door.

That said, we also have to balance that with other priorities as well.

We also care greatly about correctness. As I said above, accidental mutations were historically a consistently bad problem. I lost count of how many times I answered people's questions about "why isn't my UI updating?" and "why is the logic not working right?" that boiled down to "oops, I accidentally called array.sort() in a mapState", or "oops, I missed a nested object spread in a reducer". Immer fixed those.

We also care about DX. "Boilerplate" was the top complaint about Redux usage, and hand-written immutable update logic was one of the major reasons. Immer fixed that too.

Finally, reducer perf is not the sole aspect of perf that matters. From everything I've seen over the years, reducers are very rarely the main perf bottleneck. It's typically the cost of updating the UI that takes longer, especially as many components re-render. Along with that, as Lenz said, most Redux apps are not dispatching hundreds of actions a second, and writing hand-written reducers is still an entirely valid option if better perf is necessary. (Or, as he also suggested, copy-paste createSlice into your own codebase and modify it as needed.)

If I could wave a magic wand and make Immer have 0 overhead, I'd do that. If I could wave a magic wand and make it trivial to swap Immer for another lib or turn it off and still use createSlice, I'd at least consider it.

If you look at #3074 , we've experimented with trying to make the use of Immer configurable, which would also in theory allow swapping it with a no-op set of update functions too. But, that PR already got really complicated, because we do rely on Immer's functionality everywhere. So, it's not a trivial effort to do that swap. (And frankly we've already got a huge proliferation of "builder callbacks" and buildCreateWhatever methods, which has me concerned about both bundle size and cognitive overload for our users.)

Given all those factors, we've chosen the tradeoff of defaulting to better DX and better runtime safety, at the expense of some amount of reducer perf. That provides the most benefit to the largest number of users. (And, somewhat conveniently, also reduces the amount of time we as maintainers have to spend answering people's questions about why their code doesn't work as expected.)

So, yes, I'm always interested in ways that we can improve Redux-related perf. But we have to balance all these factors for the real world :)

Honestly, the best thing that could be done here is to investigate Immer's internals and try to understand why finalizeProperty seems to be relatively expensive in these cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants