Speed up subscription behavior by tracking state in middleware

[markerikson]

This PR:

• Rewrites our internal RTKQ subscription handling so that the primary state is kept internal to the RTKQ middleware:
  • The subscriptions data is now kept as a mutable object, initialized in the outer middleware and passed through to each "handler"
  • The batchActions file (which probably ought to be renamed) mutates that state directly for speed. (Using an Immer-powered reducer added 2-3ms of noticeable overhead for every action.) It then sets a queueMicrotask, and at the end of the tick, clones the subscriptions state, diffs it vs the previous state, and syncs it into the Redux store as Immer patches.
  • That batchActions handler also checks to see which actions should not be forwarded to the reducers, and returns a flag indicating whether the action should continue or be halted.
  • Because the store state is no longer synchronously up to date with the latest subscription data immediately, the query hooks now dispatch an internal_probeSubscription action while rendering instead of trying to read the state. This is intercepted by the middleware, so it doesn't cause any renders or visible side effects. The middleware then returns an subscriptionExists flag alongside the "should action continue" flag, so that the hook knows whether it needs to add a new subscription entry. (Frankly this is disgustingly hacky, but it's technically legal. or at least we can get away with it :) )
  • The former subscriptionSlice, which was added as state.api.subscriptions, is now a plain function that literally-no-kidding mutates the state it sees. There's still a subscriptionsSlice variable, but I'm using it just for ease of generating the action creators I need. There's also internalSubscriptionsSlice, which is a real slice reducer, but its only action is receiving the Immer patches and updating the store copy of the subscriptions state.
• Finally adds that delay util I keep copy-pasting over into a utils files and updates files to import that instead

All this disgusting hackery does have an actual benefit.

As of 1.8, rendering a fresh list of 1000 items all subscribed to the same useSomeQuery(undefined) hook took ~6700ms (eep!).

Now, it only takes about 700-800ms. That's still not good. But it's a distinct improvement.

I'm also curious if the autobatching enhancer would still be of any use here as well.

[markerikson]

💬 Saw that findLowestPollingInterval was showing up at the tail end of some of the perf capture flame charts. I don't expect this makes any real difference, but I don't think it hurts.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6975282:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration
@examples-action-listener/counter	Configuration

[phryneas]

cough

redux-toolkit/packages/toolkit/src/query/tests/helpers.tsx

Lines 39 to 41 in 988c049

	export function waitForFakeTimer(time = DEFAULT_DELAY_MS) {
	return new Promise((resolve) => setTimeout(resolve, time))
	}

Maybe we should just rename that one? 😅

[phryneas]

The batchActions file (which probably ought to be renamed) mutates that state directly for speed. (Using an Immer-powered reducer added 2-3ms of noticeable overhead for every action.) It then sets a queueMicrotask, and at the end of the tick, clones the subscriptions state, diffs it vs the previous state, and syncs it into the Redux store as Immer patches.

I'm actually not seeing this - you are still using the immerized subscriptionReducer here. Is that intended?

[phryneas]

Ugh I think I reviewed much of this on an outdated state 🤦
Trying to review an already-merged PR in VSCode is just a pain.

[phryneas]

Some tidbits here and there, but already looking very good :)

Generally we probably have to do a big cleanup on the 1.9 branch before we release it - there is quite a lot of commented-out and debugging code in there now.

[phryneas]

why did you remove that part?

[markerikson]

Ah... good question. I think it was that the test seemed more focused on "was there any state at all before, and did it all get cleared out by the reset action", rather than the specifics of exactly which state fields existed, and I was getting tired of adding delay() calls to make things line up.