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.