nowinandroid
nowinandroid copied to clipboard
[Bug]: Some `combine` works on Main thread
Is there an existing issue for this?
- [X] I have searched the existing issues
Is there a StackOverflow question about this issue?
- [X] I have searched StackOverflow
What happened?
map
method in combine
flow works on Main thread.
|
Relevant logcat output
No response
Code of Conduct
- [X] I agree to follow this project's Code of Conduct
Personally, umm... I'm not sure why flowOn
needs to be added in the Repository and UseCase. I think that specifying a dispatcher in the coroutine builder would make thread management and tracking easier.
If thread switching is implemented in the Repository and UseCase as you've modified, it seems that it could unnecessarily increase the amount of code to manage and make things more complicated.
@yongsuk44 I added this flowOn
on the Repository and UseCase because that comebine
using almost all map
method. Map
would take long time when the collection become bigger, so that the logic need to be run on Dispatchers.Default
.
I think that specifying a dispatcher in the coroutine builder would make thread management and tracking easier.
@Jaehwa-Noh 😀
This comment was left because I think that handling thread management and tracking with flowOn
at the start of the coroutine builder or logic would be efficient.
// Starting point of logic
.flowOn(Dispatchers)
.stateIn(...)
// Coroutine builder
viewModelScope.launch(Dispatcher) { ... }
viewModelScope.async(Dispatcher) { ... }
By doing this, wouldn't it be possible to remove the Dispatcher dependencies in the UseCase and repository?
Additionally, I also believe that performing complex computations on Dispatchers.Default
is efficient. However, I'm not quite sure if it's efficient to switch the current tasks to Default
. It would be effective if the project data size were very large, but in the current project, I'm unsure how much of a performance improvement there would be. Having a benchmark to compare before and after would make it easier to understand. 🤔
@yongsuk44 I don't think that repository
or usecase
own coroutine dispatcher would be controlled in viewModel
because usecase
and repository
could be re-used anywhere. Imagine that controlled by coroutineScope
in the viewModel
. When use that repository
or usecase
in another viewModel
, developer must always consider that viewModelScope
is working properly or not, and that leads them to be making mistake easily.
Current data size isn't enormous, but there's no reason we carry out the risk. I don't say this is for improve the performance, I do this for prevent ANR, long calculate on main thread easily bring that error.
There is no need to worry about ANR as long as the thread is not blocked when working on Dispatchers.Main
. Most functions in our current project are designed as suspend functions, so they should be fine.
Of course, if there are incredibly complex calculations or tasks that block the thread, it would be reasonable to switch to Dispatchers.Default
or Dispatchers.IO
. However, these operations often involve calls to Room
or Retrofit
, most of which support the suspend modifier, so I think they should be fine.
As I mentioned in a previous comment, I'm not sure if the increased amount of code that needs to be managed by adding flowOn
is worth it.
Am I missing anything here? 🤔
@yongsuk44 You can check this documentation.
Dispatchers.Main - Use this dispatcher to run a coroutine on the main Android thread. This should be used only for interacting with the UI and performing quick work. Examples include calling suspend functions, running Android UI framework operations, and updating LiveData objects.
Dispatchers.Main
is not safe for ANR. That is why Room
and Retrofit
using Dispatchers.IO
.
ANR occurs when the main thread is 'blocked' for a certain period of time. The suspension of coroutines and the blocking of threads are distinct concepts.
Additionally, when implementing Room and Retrofit with the suspend modifier, the operations are handled on the IO.