From f40060bb36d252745954da91600f3e93f918454b Mon Sep 17 00:00:00 2001 From: Habib Okanla Date: Sun, 5 Feb 2023 00:41:37 -0500 Subject: [PATCH] There are very visible perf issues during app usage and scrolling that clearly indicates that the app is doing too much on the main thread. After digging for instances where Dispatchers.Main is used, it's an easy fix to switch to Dispatchers.IO, which visibility improve perf. A few thoughts about perf considerations 1. There is no need to force Dispatchers.Main for data that is consumed as state by compose, since flows consumed as state will always flow on main, so we can use a background thread to guarantee best performance. 2. Using Dispatchers.IO is appropriate for disk/network operations to have a device-constrained thread pool that will avoid draining IO-related device resources. Using Dispatchers.Default is more appropriate for computational tasks (bitmap manipulation, delays, etc..) 3. There are a few instances of methods creating coroutine scopes in their body just to launch something that will delay. This is creating a lot of loose scopes, and you can avoid this by just moving scope creation to a class-level field and reusing it, or better yet, make your method suspending so that scope is controlled by the caller. --- .../amethyst/service/relays/RelayPool.kt | 4 +- .../amethyst/ui/screen/CardFeedViewModel.kt | 29 +++++----- .../amethyst/ui/screen/FeedViewModel.kt | 26 ++++----- .../amethyst/ui/screen/UserFeedViewModel.kt | 54 +++++++++---------- 4 files changed, 51 insertions(+), 62 deletions(-) diff --git a/app/src/main/java/com/vitorpamplona/amethyst/service/relays/RelayPool.kt b/app/src/main/java/com/vitorpamplona/amethyst/service/relays/RelayPool.kt index ee3a3670d..f4fc37874 100644 --- a/app/src/main/java/com/vitorpamplona/amethyst/service/relays/RelayPool.kt +++ b/app/src/main/java/com/vitorpamplona/amethyst/service/relays/RelayPool.kt @@ -13,6 +13,9 @@ import nostr.postr.events.Event * RelayPool manages the connection to multiple Relays and lets consumers deal with simple events. */ object RelayPool: Relay.Listener { + + val scope = CoroutineScope(Job() + Dispatchers.IO) + private var relays = listOf() private var listeners = setOf() @@ -116,7 +119,6 @@ object RelayPool: Relay.Listener { val live: RelayPoolLiveData = RelayPoolLiveData(this) private fun refreshObservers() { - val scope = CoroutineScope(Job() + Dispatchers.Main) scope.launch { live.refresh() } diff --git a/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/CardFeedViewModel.kt b/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/CardFeedViewModel.kt index 2eeb14032..947c46cce 100644 --- a/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/CardFeedViewModel.kt +++ b/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/CardFeedViewModel.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext class CardFeedViewModel(val dataSource: NostrDataSource): ViewModel() { private val _feedContent = MutableStateFlow(CardFeedState.Loading) @@ -29,11 +30,8 @@ class CardFeedViewModel(val dataSource: NostrDataSource): ViewModel() { private var lastNotes: List? = null - fun refresh() { - val scope = CoroutineScope(Job() + Dispatchers.Default) - scope.launch { - refreshSuspended() - } + suspend fun refresh() = withContext(Dispatchers.IO) { + refreshSuspended() } private fun refreshSuspended() { @@ -83,19 +81,16 @@ class CardFeedViewModel(val dataSource: NostrDataSource): ViewModel() { return (reactionCards + boostCards + textNoteCards).sortedBy { it.createdAt() }.reversed() } - fun updateFeed(notes: List) { - val scope = CoroutineScope(Job() + Dispatchers.Main) - scope.launch { - val currentState = feedContent.value + private fun updateFeed(notes: List) { + val currentState = feedContent.value - if (notes.isEmpty()) { - _feedContent.update { CardFeedState.Empty } - } else if (currentState is CardFeedState.Loaded) { - // updates the current list - currentState.feed.value = notes - } else { - _feedContent.update { CardFeedState.Loaded(mutableStateOf(notes)) } - } + if (notes.isEmpty()) { + _feedContent.update { CardFeedState.Empty } + } else if (currentState is CardFeedState.Loaded) { + // updates the current list + currentState.feed.value = notes + } else { + _feedContent.update { CardFeedState.Loaded(mutableStateOf(notes)) } } } diff --git a/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/FeedViewModel.kt b/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/FeedViewModel.kt index 54d642ab0..f064441fa 100644 --- a/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/FeedViewModel.kt +++ b/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/FeedViewModel.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import nostr.postr.events.TextNoteEvent +import java.util.concurrent.atomic.AtomicBoolean class NostrChannelFeedViewModel: FeedViewModel(NostrChannelDataSource) class NostrChatRoomFeedViewModel: FeedViewModel(NostrChatRoomDataSource) @@ -86,25 +87,21 @@ abstract class FeedViewModel(val dataSource: NostrDataSource): ViewModel() } fun refresh() { - viewModelScope.launch(Dispatchers.Default) { + viewModelScope.launch(Dispatchers.IO) { val notes = newListFromDataSource() val oldNotesState = feedContent.value if (oldNotesState is FeedState.Loaded) { if (notes != oldNotesState.feed) { - withContext(Dispatchers.Main) { - updateFeed(notes) - } - } - } else { - withContext(Dispatchers.Main) { updateFeed(notes) } + } else { + updateFeed(notes) } } } - fun updateFeed(notes: List) { + private fun updateFeed(notes: List) { val currentState = feedContent.value if (notes.isEmpty()) { @@ -117,19 +114,18 @@ abstract class FeedViewModel(val dataSource: NostrDataSource): ViewModel() } } - var handlerWaiting = false - fun invalidateData() { - synchronized(handlerWaiting) { - if (handlerWaiting) return + private var handlerWaiting = AtomicBoolean() + @Synchronized + private fun invalidateData() { + if (handlerWaiting.get()) return - handlerWaiting = true + handlerWaiting.set(true) val scope = CoroutineScope(Job() + Dispatchers.Default) scope.launch { delay(100) refresh() - handlerWaiting = false + handlerWaiting.set(false) } - } } private val cacheListener: (LocalCacheState) -> Unit = { diff --git a/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt b/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt index ad85ee181..cbe938f54 100644 --- a/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt +++ b/app/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt @@ -22,6 +22,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import java.util.concurrent.atomic.AtomicBoolean class NostrUserProfileFollowsUserFeedViewModel(): UserFeedViewModel( NostrUserProfileFollowsDataSource @@ -39,13 +41,11 @@ open class UserFeedViewModel(val dataSource: NostrDataSource): ViewModel() private val _feedContent = MutableStateFlow(UserFeedState.Loading) val feedContent = _feedContent.asStateFlow() - fun refresh() { - val scope = CoroutineScope(Job() + Dispatchers.Default) - scope.launch { - refreshSuspended() - } + suspend fun refresh() = withContext(Dispatchers.IO) { + refreshSuspended() } + private fun refreshSuspended() { val notes = dataSource.loadTop() @@ -59,34 +59,30 @@ open class UserFeedViewModel(val dataSource: NostrDataSource): ViewModel() } } - fun updateFeed(notes: List) { - val scope = CoroutineScope(Job() + Dispatchers.Main) - scope.launch { - val currentState = feedContent.value - - if (notes.isEmpty()) { - _feedContent.update { UserFeedState.Empty } - } else if (currentState is UserFeedState.Loaded) { - // updates the current list - currentState.feed.value = notes - } else { - _feedContent.update { UserFeedState.Loaded(mutableStateOf(notes)) } - } + private fun updateFeed(notes: List) { + val currentState = feedContent.value + if (notes.isEmpty()) { + _feedContent.update { UserFeedState.Empty } + } else if (currentState is UserFeedState.Loaded) { + // updates the current list + currentState.feed.value = notes + } else { + _feedContent.update { UserFeedState.Loaded(mutableStateOf(notes)) } } } - var handlerWaiting = false - fun invalidateData() { - synchronized(handlerWaiting) { - if (handlerWaiting) return + var handlerWaiting = AtomicBoolean() - handlerWaiting = true - val scope = CoroutineScope(Job() + Dispatchers.Default) - scope.launch { - delay(100) - refresh() - handlerWaiting = false - } + @Synchronized + private fun invalidateData() { + if (handlerWaiting.get()) return + + handlerWaiting.set(true) + val scope = CoroutineScope(Job() + Dispatchers.Default) + scope.launch { + delay(100) + refresh() + handlerWaiting.set(false) } }