Bug 1877307 - RFC: Customisation of the Store dispatcher

docs/contributing.md

@@ -45,6 +45,7 @@ Before contributing, please review our [Community Participation Guidelines](http
 * [0006 - Determining the set of default search engines]({{ site.baseurl }}/rfc/0006-search-defaults)
 * [0007 - Synchronizing the branching and versioning of Android Components with the Mozilla release trains]({{ site.baseurl }}/rfc/0007-synchronized-releases)
 * [0008 - Adding tab partitions (groups) to BrowserState]({{ site.baseurl }}/rfc/0008-tab-groups)
+* [0012 - Allow customisation of the Store dispatcher]({{ site.baseurl }}/rfc/0012-customizable-store-dispatcher)
 
 ### Presentations
 

docs/rfcs/0012-customizable-store-dispatcher.md

@@ -0,0 +1,100 @@
+---
+layout: page
+title: Allow customisation of the Store dispatcher
+permalink: /rfc/0012-customizable-store-dispatcher
+---
+
+* Start date: 2024-01-29
+* RFC PR: [#5353](https://github.com/mozilla-mobile/firefox-android/pull/5353)
+
+## Summary
+
+In most applications of the `Store`, it is preferable to have reducers perform work on the main thread. This helps to simplify the levels of async tasks that need to be synchronised with other async tasks that are already on the main thread.
+
+## Motivation
+
+Android embedders use the main thread for UI, user-facing, or touch handling work. For example, notifying UI components when IO from storage layers have completed, an engine's tasks that can happen on a separate thread, or global-level state updates for different components to observe.
+
+When components dispatch actions to the store, these actions are performed on an independant single thread dispatcher in the `Store` to avoid overloading the main thread with heavy work that might be performed during the `reduce` or in a `Middleware`. In practice, these actions have been short and fast so they do not cause overhead. Work done in a `Middleware` that can be slow, like I/O,  is put onto separate Dispatchers. The performance optimization to switch to a `Store` thread, requires that components which are always run on the main thread, to ensure synchronisation is now kept between the main thread and the store thread for observers of the `State`.
+
+There are some advantages to this change:
+
+* Simplicity for main-thread `Store`s.
+* Unit testing can now occur on the test framework's thread.
+* Fewer resources needed for context shifting between threads.
+
+For an example of thread simplicity, an `Engine` typically has its own 'engine thread' to perform async work and post/request results to the main thread (these APIs are identified with the `@UiThread` annotation). Once we get the callback for those results, we then need to dispatch an action to the store that will then happen on a Store thread. Features then observe for state changes and then make UI changes on the main thread. This switching of threads can be reduced. A simplified form of this thread context switching can be seen in the example below:
+
+```kotlin
+// engine thread
+engineView.requestApiResult { result ->
+  store.dispatch(UpdateResultAction(result))
+}
+
+// store thread
+fun reduce(state: State, action: Action) {
+  is UpdateResultAction -> {
+    // do things here.
+  }
+}
+
+// store thread
+Middleware {
+  override fun invoke(
+    context: MiddlewareContext<State, Action>,
+    next: (Action) -> Unit,
+    action: Action,
+  ) {
+    // perform side-effects that also happen on the store thread.
+  }
+}
+
+// main thread
+store.flowScoped { flow ->
+  flow.collect {
+    MainScope().launch {
+      // perform work on the main thread.
+    }
+  }
+}
+```
+
+
+
+## Guide-level explanation
+
+There are two possible ways to allow for this behaviour during the store creation:
+
+1. Pass in a `CoroutineDispatcher`.
+2. Configurable set of pre-defined options to choose.
+
+Passing in a `CoroutineDispatcher` has the flexibility to allow any preferred dispatcher to be used (e.g. `Dispatchers.IO` or Android Component's `Dispatchers.Cached`),  however that could lead to complications on choosing the wrong dispatcher. Ensuring that testing is also done with the same dispatcher configuration can also be difficult with this pattern.
+
+With a predefined set of options, we have a smaller surface area of changes that we want to maintain, possibly starting off with either the main dispatcher or the current single threaded dispatcher. We can continue to use our `StoreThreadFactory` and disable it when we switch to the `Main` dispatcher.
+
+Below are examples of what a possible API surface would look like to a consumer of an "AppStore":
+
+```kotlin
+// Configurable set of pre-defined options to choose.
+class AppStore(
+    initialState: AppState = AppState(),
+) : Store<AppState, AppAction>(
+    initialState = initialState,
+    reducer = AppStoreReducer::reduce,
+    runOnMain = true, // a boolean or enum value to signify the dispatcher used.
+)
+```
+
+Another advantage of this change is that `Store`s would be able to run on the test framework's default dispatcher. This reduces the number of intermittent test failures we have seen where the `Store` is still processing actions async to an assertion. Fewer usages of `store.waitUntilIdle()` are required after every `dispatch` is called in a test, as a result.
+
+## Drawbacks
+
+* Mistakenly doing work on the main thread - we could end up performing large amounts of work on the main thread unintentionally if we are not careful. This could be because of a large number of small tasks, a single large task, a blocking task, or a combination. This could be mitigated by keeping the `Store` dispatcher as the default, and catching performance regressions in the specific features they are used in.
+
+## Rationale and alternatives
+
+An alternative is to not change the current state and require each `Store` to individually decide how it wants to handle the multiple levels of async tasks.
+
+## Unresolved questions
+
+* While performance gains are not an explicit intent, there is a theoretical advantage, but not one we will pursue as part of this RFC. How much would we save, if any?