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

Dispatch actions and run reducers on background thread? #92

Closed
haifengkao opened this issue Dec 22, 2020 · 4 comments
Closed

Dispatch actions and run reducers on background thread? #92

haifengkao opened this issue Dec 22, 2020 · 4 comments

Comments

@haifengkao
Copy link
Collaborator

I saw SwiftRex always use main thread to run all actions and reducers (in ReduxPipelineWrapper)
For performance reason, are there any ways to avoiding using main thread?

@luizmb
Copy link
Member

luizmb commented Dec 22, 2020

Excellent question. It's really important that this runs on Main Queue. State mutation is exactly the thing you want to run in the main thread, this ensures consistency of multiple incoming actions, and consistency of UI. Your UI is a function of that state, they MUST be in the same thread. Although you could, theoretically, run the reducers in another serial queue and only publish to State Publisher on main queue when reducer is done, there's no real benefit in doing that. Reducer is a pure and sync func, that doesn't perform side-effects or expensive work.

Another point to consider is animations, when UI dispatches an action that will be animatable, you can dispatch it inside a withAnimation block, such as:

withAnimation {
     viewModel.dispatch(.tapButtonThatWillFadeSomethingOut)
}

With that code, the State will be reduced in the very same Main Queue Run Loop, StatePublisher will dispatch correctly to perform the animation and you will have the expected behaviour of fading out other elements that are eventually hidden by flags in your AppState. Same for drag gestures.

The optimisation recommended, tho, is to make use of inout State of reducers, and "vars" in your AppState, to avoid copy of arrays (copy-on-modify behaviour) when dealing with larger arrays (maybe more than 100 items, or 50 if the items are really expensive). Another thing that I recommend is keeping Images off the AppState, by using NSCache and keeping on AppState only keys to NSCache and a hash with the current version, I'll try to cover this technique in a further article).

ReduxPipelineWrapper is expected to be initiated from the Main Queue as well (together with the Store), so we collect the Queue ID here:
https://github.com/SwiftRex/SwiftRex/blob/develop/Sources/SwiftRex/CoreTypes/StoreImplementations/ReduxPipelineWrapper.swift#L25

To be used in the "asap" extension here:
https://github.com/SwiftRex/SwiftRex/blob/develop/Sources/SwiftRex/CoreTypes/StoreImplementations/ReduxPipelineWrapper.swift#L68

This ensures that actions coming from the UI will be immediately reduced in the same RunLoop in case we are already in the main queue, which is very likely from UI events (https://github.com/SwiftRex/SwiftRex/blob/develop/Sources/SwiftRex/Foundation/DispatchQueue.swift#L19). Actions coming from Middleware will always be dispatched async to main queue, so they go to the end of the queue and follow the FIFO order. This also avoids that between a "before reducer" and "after reducer" of certain middleware, there are 2 actions changing the state.

Please feel free to play with these things and suggest possible improvements, but please keep in mind all these important protections that avoid race conditions or inconsistencies.

@haifengkao
Copy link
Collaborator Author

It's reasonable, thanks
I was trying to use inout state reducer as you suggested, but it is not included in the latest release (0.7.8)
Will it be released soon?

@luizmb
Copy link
Member

luizmb commented Dec 23, 2020

I'm gonna try to finish the tests during these holidays, perhaps postpone documentation to 0.9 or 1.0. But code-wise current develop is expected to be next release, unless the tests uncover some weird bug.

@luizmb
Copy link
Member

luizmb commented Jan 5, 2021

Version released.

I'm closing the issue now as it seems to be reasonably answered. Please reopen in case there are still more questions.

@luizmb luizmb closed this as completed Jan 5, 2021
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

2 participants