Reducers without immer

[romgrk]

Hey,

I'm aware of the position of the maintainers regarding disabling immer, as stated in #242 and #183. However, both of those discussions are the top google results when searching redux toolkit without immer. After some time, I realized that I could both keep RTK and add additional reducers without immer, which is required in edge cases for performance reasons*. I've created a gist with the solution, if you're interested it could be nice to add the gist to the end of the discussion in #242, as that is the top-level result and I'm sure many people have been landing there without a clear way to solve their issue.

Here is the link: https://gist.github.com/romgrk/bb16d7b8d3827481d04eb535e8d1bc74

This issue can be closed.

* I'm not sure what goes on inside immer's produce(), but in the edge-case I encountered, using a raw reducer took the runtime of a single action from ~200ms to ~2ms.

[phryneas]

Honestly, I'd not go so far. Check what slows it down instead. Usually those are reducers with a lot (thousands or tens of thousands) reads on the state proxy. In that case, just do one call to const originalState = original(state), do normal immutable updates with that and just return the result.

[romgrk]

Ok, interesting. It would also be nice to have that information as a summary of #242, and in the docs. The docs talk at length about how immer won't have that big of a performance cost... but no word about what to do if there is a performance cost.

[phryneas]

Well, so far we had probably two reports of performance costs ^^ (and a lot of people just generally being against immer)

But if you want to dive in a bit and make a writeup to add for the docs, a PR would be welcome :)

[romgrk]

Ok sure, I'll send a PR for the docs in the next days.

About the original() from immer, is that function exposed to the users? Does the user need to dabble with peer dependencies or something? If you have a short demo or snippet it would help me test it out.

[EskiMojo14]

About the original() from immer, is that function exposed to the users? Does the user need to dabble with peer dependencies or something? If you have a short demo or snippet it would help me test it out.

yes, it's one of the functions RTK re-exports.

[romgrk]

Alright, I've run some tests with both solutions, unfortunately original() doesn't seem to improve the performance.

Here is the code being tested: https://gist.github.com/romgrk/6aac91df09cdebf8a0761b9fb3afd20e (lensPath and over functions from rambda (not ramda)). Would you please review the solution with original() to confirm I'm using properly?

The result table below shows an example run for each solution. There has been some variation in the profiling run times, but the orders of magnitude stay the same. Any solution with immer is in the hundreds of ms range, and the solution without immer is in the 0-1ms range.

Name	Run time
With immer	760.95ms
With immer + original	770.74ms
Without immer	0.16ms

Immer

Immer+original

No immer

[EskiMojo14]

instead of

      state.byId[id].metrics =
        state.byId[id].metrics?.map(metric =>
          metric.id !== metricId ?
            metric :
            { ...metric, ...newMetric }
        )

can you try doing

  const metric = state.byId[id].metrics?.find((metric) => metric.id === metricId);
  if (metric) {
    Object.assign(metric, newMetric);
  }

[phryneas]

Would be interesting to see the dataset here, too.

[romgrk]

I've run again with the proposition from @EskiMojo14, it stays in the same order of magnitude. For comparison, I'm including profiling for both the original and the proposition.

Name	Run time
Immer + .map()	153.59ms
Immer + Object.assign()	153.74ms

So the performance is identical.
I can't share the dataset but think multi-MB JSON.

[romgrk]

The issue with immer is that for some reason, the performance is O(n) insead of O(1) as it should be. My guess is immer does some kind of deep check on new properties, so it visits everything that is added by the newMetric content.

[markerikson]

Can you share a fully running sandbox or repo that shows this behavior, with a meaningful dataset?

Also, note that Immer does recursively freeze data by default. Can't tell if that's the issue here because there's not enough of the perf capture shown in those screenshots. However, if that is the issue, note that you can do an Object.freeze(value) on the top-level object to prevent Immer from recursing:

https://immerjs.github.io/immer/freezing

[romgrk]

Freezing the data brings the performance to the same level as not using immer. I'll send a PR to add the detail to the doc.
I know it's been said enough, but I really wish there was a way to use this module without immer; it adds too much magic to be comfortable to use.

[drop-george]

@romgrk what do you mean by freezing the data? Are you perhaps calling Object.freeze on something?

I'm running into a similar performance issue which I suspect has to do with immer.

[phryneas]

@drop-george return Object.freeze(whatYouWantToReturn)

[markerikson]

Ah, forgot this was open :)

[caseycole589]

I also had this performance problem. Using freeze fixed the problem for any one that runs across this issue.

[gentlee]

@markerikson so what's the action item here?

1. Disable immer auto freeze [in prod] by default? Enable immer auto freeze only in dev by default?
2. Make immer a peer dependency and/or option to disable it?

I guess everyone agrees that there neither should not be such issues with performance, nor everyone should read the whole doc for immer and enable/disable autofreeze, or freeze states manually to be able to use RTK efficiently.

Currently I don't see anything in the docs about that problem and how to fix it.

[markerikson]

@gentlee no plans to change any of the behavior here. Do you have specific perf traces showing that freezing is an issue atm?

[gentlee]

@markerikson no I'm just looking at this conversation and at our codebase where we added these lines some time ago:

// Performance optimization for Immer by disabling auto freeze in other envs than dev
// https://github.com/reduxjs/redux-toolkit/issues/2405
// https://immerjs.github.io/immer/freezing
setAutoFreeze(__DEV__)

As I see in this thread are already some traces and perf metrics used, but solution is not included into any docs / PRs. So I assume the problem still exists.

PS. Also writing some article with state manager comparison and just can't avoid this issue with additional complexity with configuring immer, would be much better [for redux] if it did not exist.

[markerikson]

@gentlee I'd recommend actually doing some perf benchmarking to see if it is really an issue in your app's case.

Use of Immer (and its freezing behavior) is the right default behavior to prevent common bugs. Accidental mutations were the number one cause of Redux bugs prior to RTK, and since then they've been effectively eliminated. Worst case, as your snippet there shows, freezing can be disabled if absolutely necessary.

Also, note that this issue is multiple years old. RTK 2.0 includes Immer 10, which had some noticeable perf improvements.

[gentlee]

@markerikson I'll do some a bit later, just out of interest.

About redux bugs - I'm sure these kind of bugs are mostly happening only during the initial development of some feature and trying to understand why it doesn't work. It is hardly possible to make something work properly with mutations - subscriptions won't work.

So for sure such things help during development, but unnecessary in prod.

Most common bugs from my experience is breaking memoizations by returning [] or new objects / functions from not memoized selectors. I even proposed this some time ago.

[markerikson]

@gentlee I've spent 8+ years helping people with Redux, and answered thousands of questions. Trust me when I say that pre-RTK, accidental mutations were absolutely the most common bug :)

[phryneas]

There's one more thing to consider: afaik, the auto-freezing variant of immer became at some version faster than the non-auto-freezing variant.
If I remember correctly, the freezing made it possible to have some assumptions which sped up the whole algorithm.

So at that point it's "immer" vs "not immer", and believe me, to this day we see enough broken code that we won't budge on that.
If you need exceptional performance in one specific edge case reducer, you can simply opt out for that one bit and write that one reducer by hand.
There would be enough people otherwise who are lead by a random badly-explained blog post outside of our control to turn immer off for "performance" without actually knowing of the implications, heavily shooting themselves in the foot.

---

PS: regarding freezing and performance see this release and all the linked threads: https://github.com/immerjs/immer/releases/tag/v8.0.0

[gentlee]

@markerikson @phryneas added benchmark in separate issue, got very confusing results.

Shortly - performance is x100 slower at average and neither disabling auto freeze nor using original help much.