Skip calling subscribers when an action doesn't change the state at all

[jkillian]

New Features

Skip notifying subscribers when a dispatched action doesn't change the current state at all

What is the new or updated feature that you are suggesting?

Currently, when a dispatch happens, all subscribers (aka listeners) are called.

However, instead, a small performance optimization could be added that only calls listeners if the new state is not referentially equal to the previous state. This could be implemented with just a few extra lines of code in the current dispatch method.

While not necessarily the ideal code, this feature could be implemented as simply as something like this:

     try {
       isDispatching = true
-      currentState = currentReducer(currentState, action)
+      const prevState = currentState;
+      currentState = currentReducer(prevState, action)
+      if (prevState === currentState) {
+        return action
+      }
     } finally {
       isDispatching = false
     }

Why should this feature be included?

The main motivation here is as a performance optimization. If an action changes no state, there's no point to re-call subscribers. This changes lines up quite nicely with the architecture and philosophy of redux: "The intended guarantee is that Redux eventually calls all subscribers with the most recent state available, but not that it always calls each subscriber for each action." In applications that dispatch many actions only of which a few lead to state changes, this performance difference can be quite noticeable.

Possible (imo invalid) objections:

One objection might be that an action that changes no state shouldn't be dispatched in the first place. However, as reducer implementation is a black box from the point of view of an action dispatcher, I don't think it's fair for code at the dispatch site to have to determine if a state change will happen or not.

Another objection might be that this functionality can easily be implemented as a store enhancer, so why change redux core? However, in my opinion, implementing a store enhancer to do this is fairly complex because it ends up needing to overwrite so much of the core redux store logic (related #3836 (comment)).

A third objection would be that doing a top-level referential equality check on state would mess things up for people who aren't handling data in the store immutably. This is true, however, immutability is required by redux so this isn't a usecase that we need to worry about.

A fourth objection would be that adding an extra conditional to redux's dispatch method would decrease performance for actions that do actually modify state. While this is true, I would think this effect would be so small that it isn't necessary to consider.

Actual downsides:

While I don't think the above objections are valid, there are a few actual downsides that I can imagine. The biggest one is that I'm not sure this change would have the intended effect when redux dev tools are in use. IIUC, redux devtools enhance the store and end up storing a lot of extra metadata in the state (invisibly to the application). So for actions that don't modify application state at all, redux devtools might still modify its extra part of the state, causing the proposed performance optimization to have no affect.

The other possible downside is simply that this is a slight change in existing behavior. While it seems harmless to me, I can't be sure if people are somehow relying on actions with no state changes to still call subscribers.

Questions

So all of that said, two things I'd be curious about: 1) if this change seems like a good one, and 2) if so, if the concern about redux devtools mentioned above is correct and if it would need a workaround or not. Thanks!

[timdorr]

There are many use cases where dispatching doesn't update state, but still wants to notify subscribers. This is most common with things that have side effects outside of the dispatch that need to be "cleaned up" after the dispatch completes.

The philosophy quote you have above is in reference to store enhancers or middleware that delays the dispatch, such as when waiting for a network request. It's not intended as a performance optimization, just a reality of how Redux might be used.

If this kind of behavior is wanted, we leave that up to the user, because where you place the de-duplication code is very relevant. Having it in a specific place in your middleware stack could be very important to other middleware that relies on duplicates going through to subscribers. We can't break that behavior at the very core dispatch level, because it becomes impossible to override. Instead, we'll continue to leave this in users' hands.

[markerikson]

I think there's likely a few reasons to want to keep the existing behavior, including the fact that it is how the logic works atm, and that things like the DevTools need to track all actions. I haven't looked at the DevTools enhancer logic in a while, so it's very possible that the tracking behavior there happens regardless of whether any subscribers were notified.

I think another question here is what the subscription notifications mean conceptually. I've described the Redux store subscription system as "an event emitter with a single event: 'some action was dispatched' ". Note that that's a subtle difference from "the Redux state was updated". Now, yes, the vast majority of dispatched actions result in state updates, and the first thing almost every subscriber does is call store.getState() and diff the results, but there's some difference in semantics here.

In theory, no-op actions should result in subscribers bailing out very quickly. For example, in React-Redux, each useSelector hook results in 4 function calls per dispatch:

• subscriber()
• store.getState()
• selector(state)
• equalityFn(prevResult, currentResult)

https://github.com/reduxjs/react-redux/blob/v7.2.4/src/hooks/useSelector.js#L67-L72

That's a non-zero amount of effort, but it's also logic that shouldn't take very long to run in most situations.

(Interestingly, I note that we don't appear to have a if (currentState === prevState) return bailout here, while we do have one in connect.)

[jkillian]

Thanks for the detailed responses - makes sense that such a change would be overly breaking (even regardless of if it makes sense semantically or not).

(Interestingly, I note that we don't appear to have a if (currentState === prevState) return bailout here, while we do have one in connect.)

That is interesting - perhaps that's a more narrowly-scoped optimization that would make sense. I suppose it could still be a bit breaking, if you had a useSelector(state => state, () => false) call for some reason, but seems like it would be a reasonable improvement in many cases. It would definitely help in the cases where the passed selector or equality function weren't especially trivial (which probably happens more often than it should I expect!)

[vytenisu]

Another possible optimization which increased performance on one of the projects I worked on dramatically - throttling notifications from Redux and marking certain actions as "passive" (state-changing only) causing them to update state if needed but not notify.

It is something to be used with care because essentially it breaks default lifecycle of Redux and should not be included in default Redux functionality. But still, maybe someone will find it worth to try:

https://www.npmjs.com/package/redux-notification-enhancer