RoomList: avoid to many recompositions

2022-10-30 13:48:06 +01:00
parent e3845bc976
commit 207e402cc5
8 changed files with 88 additions and 41 deletions
--- a/features/roomlist/src/main/java/io/element/android/x/features/roomlist/RoomListScreen.kt
+++ b/features/roomlist/src/main/java/io/element/android/x/features/roomlist/RoomListScreen.kt
@@ -16,53 +16,57 @@ import androidx.compose.ui.unit.sp
 import com.airbnb.mvrx.Success
 import com.airbnb.mvrx.compose.collectAsState
 import com.airbnb.mvrx.compose.mavericksViewModel
+import io.element.android.x.core.data.LogCompositions
 import io.element.android.x.designsystem.components.Avatar
+import io.element.android.x.features.roomlist.model.MatrixUser
 import io.element.android.x.matrix.core.RoomId
 import io.element.android.x.matrix.room.RoomSummary

@Composable
 fun RoomListScreen(
-    viewModel: RoomListViewModel = mavericksViewModel(),
    onSuccessLogout: () -> Unit = { },
    onRoomClicked: (RoomId) -> Unit = { }
 ) {
-    val state by viewModel.collectAsState()
-    if (state.logoutAction is Success) {
+    val viewModel: RoomListViewModel = mavericksViewModel()
+    val logoutAction by viewModel.collectAsState(RoomListViewState::logoutAction)
+    if (logoutAction is Success) {
        onSuccessLogout()
        return
    }
+    LogCompositions(tag = "RoomListScreen", msg = "Root")
+    val roomSummaries by viewModel.collectAsState(RoomListViewState::rooms)
+    val matrixUser by viewModel.collectAsState(RoomListViewState::user)
    RoomListContent(
-        state = state,
+        roomSummaries = roomSummaries().orEmpty(),
+        matrixUser = matrixUser,
        onRoomClicked = onRoomClicked,
-        onLogoutClicked = {
-            viewModel.handle(RoomListActions.Logout)
-        }
+        onLogoutClicked = viewModel::logout
    )
 }

@Composable
 fun RoomListContent(
-    state: RoomListViewState,
+    roomSummaries: List<RoomSummary>,
+    matrixUser: MatrixUser,
    onRoomClicked: (RoomId) -> Unit,
    onLogoutClicked: () -> Unit,
 ) {
+    LogCompositions(tag = "RoomListScreen", msg = "Content")
    Surface(color = MaterialTheme.colorScheme.background) {
        Column(
            modifier = Modifier.fillMaxSize()
        ) {
            RoomListTopBar(
-                state = state,
+                matrixUser = matrixUser,
                onLogoutClicked = onLogoutClicked
            )
-            val rooms = state.rooms
-            if (rooms is Success) {
-                LazyColumn {
-                    items(rooms()) { room ->
-                        RoomItem(room = room) {
-                            onRoomClicked(it)
-                        }
+            LazyColumn {
+                items(roomSummaries, key = { it.identifier() }) { room ->
+                    RoomItem(room = room) {
+                        onRoomClicked(it)
                    }
                }
+
            }
        }
    }
@@ -70,14 +74,14 @@ fun RoomListContent(

@OptIn(ExperimentalMaterial3Api::class)
@Composable
-fun RoomListTopBar(state: RoomListViewState, onLogoutClicked: () -> Unit) {
+fun RoomListTopBar(matrixUser: MatrixUser, onLogoutClicked: () -> Unit) {
+    LogCompositions(tag = "RoomListScreen", msg = "TopBar")
    TopAppBar(
        title = {
            Row(
                modifier = Modifier.fillMaxWidth(),
                verticalAlignment = Alignment.CenterVertically
            ) {
-                val matrixUser = state.user
                Avatar(data = matrixUser.avatarData)
                Spacer(modifier = Modifier.width(8.dp))
                Text("${matrixUser.username}")
--- a/features/roomlist/src/main/java/io/element/android/x/features/roomlist/RoomListViewModel.kt
+++ b/features/roomlist/src/main/java/io/element/android/x/features/roomlist/RoomListViewModel.kt
@@ -4,6 +4,7 @@ import com.airbnb.mvrx.Fail
 import com.airbnb.mvrx.Loading
 import com.airbnb.mvrx.MavericksViewModel
 import com.airbnb.mvrx.Success
+import io.element.android.x.features.roomlist.model.MatrixUser
 import io.element.android.x.matrix.MatrixClient
 import io.element.android.x.matrix.MatrixInstance
 import kotlinx.coroutines.launch
@@ -25,6 +26,10 @@ class RoomListViewModel(initialState: RoomListViewState) :
        }
    }

+    fun logout(){
+        handleLogout()
+    }
+
    private fun handleInit() {
        viewModelScope.launch {
            val client = getClient()
--- a/features/roomlist/src/main/java/io/element/android/x/features/roomlist/RoomListViewState.kt
+++ b/features/roomlist/src/main/java/io/element/android/x/features/roomlist/RoomListViewState.kt
@@ -3,9 +3,8 @@ package io.element.android.x.features.roomlist
 import com.airbnb.mvrx.Async
 import com.airbnb.mvrx.MavericksState
 import com.airbnb.mvrx.Uninitialized
+import io.element.android.x.features.roomlist.model.MatrixUser
 import io.element.android.x.matrix.room.RoomSummary
-import org.matrix.rustcomponents.sdk.Room
-import org.matrix.rustcomponents.sdk.UpdateSummary

 data class RoomListViewState(
    val user: MatrixUser = MatrixUser(),
--- a/features/roomlist/src/main/java/io/element/android/x/features/roomlist/model/MatrixUser.kt
+++ b/features/roomlist/src/main/java/io/element/android/x/features/roomlist/model/MatrixUser.kt
@@ -1,5 +1,8 @@
-package io.element.android.x.features.roomlist
+package io.element.android.x.features.roomlist.model

+import androidx.compose.runtime.Stable
+
+@Stable
 data class MatrixUser(
    val username: String? = null,
    val avatarUrl: String? = null,
--- a/libraries/core/src/main/java/io/element/android/x/core/data/LogCompositions.kt
+++ b/libraries/core/src/main/java/io/element/android/x/core/data/LogCompositions.kt
@@ -0,0 +1,22 @@
+package io.element.android.x.core.data
+
+import android.util.Log
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.SideEffect
+import androidx.compose.runtime.remember
+import io.element.android.x.core.BuildConfig
+
+
+// Note the inline function below which ensures that this function is essentially
+// copied at the call site to ensure that its logging only recompositions from the
+// original call site.
+@Composable
+inline fun LogCompositions(tag: String, msg: String) {
+    if (BuildConfig.DEBUG) {
+        val ref = remember { Ref(0) }
+        SideEffect { ref.value++ }
+        Log.d(tag, "Compositions: $msg ${ref.value}")
+    }
+}
+
+class Ref(var value: Int)
--- a/libraries/matrix/src/main/java/io/element/android/x/matrix/MatrixClient.kt
+++ b/libraries/matrix/src/main/java/io/element/android/x/matrix/MatrixClient.kt
@@ -1,6 +1,5 @@
 package io.element.android.x.matrix

-import android.util.Log
 import io.element.android.x.core.data.CoroutineDispatchers
 import io.element.android.x.matrix.core.UserId
 import io.element.android.x.matrix.room.RoomSummaryDataSource
@@ -33,13 +32,13 @@ class MatrixClient internal constructor(

    private val slidingSyncObserver = object : SlidingSyncObserver {
        override fun didReceiveSyncUpdate(summary: UpdateSummary) {
-            Timber.v("didReceiveSyncUpdate=$summary")
+            Timber.v("didReceiveSyncUpdate=$summary on Thread: ${Thread.currentThread()}")
            roomSummaryDataSource.updateRoomsWithIdentifiers(summary.rooms)
        }
    }

    private val slidingSyncView = SlidingSyncViewBuilder()
-        .timelineLimit(limit = 10u)
+        .timelineLimit(limit = 1u)
        .requiredState(requiredState = listOf(RequiredState(key = "m.room.avatar", value = "")))
        .name(name = "HomeScreenView")
        .syncMode(mode = SlidingSyncMode.FULL_SYNC)
@@ -65,6 +64,7 @@ class MatrixClient internal constructor(
    }

    fun stopSync() {
+        roomSummaryDataSource.stopSync()
        slidingSync.setObserver(null)
        slidingSyncObserverToken?.cancel()
    }
@@ -73,6 +73,7 @@ class MatrixClient internal constructor(

    override fun close() {
        stopSync()
+        roomSummaryDataSource.close()
        client.setDelegate(null)
    }

--- a/libraries/matrix/src/main/java/io/element/android/x/matrix/room/RoomSummary.kt
+++ b/libraries/matrix/src/main/java/io/element/android/x/matrix/room/RoomSummary.kt
@@ -2,6 +2,7 @@ package io.element.android.x.matrix.room

 import io.element.android.x.matrix.core.RoomId

+
 sealed interface RoomSummary {
    data class Empty(val identifier: String) : RoomSummary
    data class Filled(val details: RoomSummaryDetails) : RoomSummary
--- a/libraries/matrix/src/main/java/io/element/android/x/matrix/room/RoomSummaryDataSource.kt
+++ b/libraries/matrix/src/main/java/io/element/android/x/matrix/room/RoomSummaryDataSource.kt
@@ -5,14 +5,14 @@ import io.element.android.x.matrix.core.RoomId
 import io.element.android.x.matrix.room.message.RoomMessageFactory
 import io.element.android.x.matrix.sync.roomListDiff
 import io.element.android.x.matrix.sync.state
-import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.SupervisorJob
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.cancelChildren
+import kotlinx.coroutines.flow.*
 import org.matrix.rustcomponents.sdk.*
+import timber.log.Timber
+import java.io.Closeable
 import java.util.*

 interface RoomSummaryDataSource {
@@ -24,7 +24,7 @@ internal class RustRoomSummaryDataSource(
    private val slidingSyncView: SlidingSyncView,
    private val coroutineDispatchers: CoroutineDispatchers,
    private val roomMessageFactory: RoomMessageFactory = RoomMessageFactory(),
-) : RoomSummaryDataSource {
+) : RoomSummaryDataSource, Closeable {

    private val coroutineScope = CoroutineScope(SupervisorJob() + coroutineDispatchers.io)

@@ -33,6 +33,7 @@ internal class RustRoomSummaryDataSource(

    init {
        slidingSyncView.roomListDiff()
+            .buffer(50)
            .onEach { diff ->
                updateRoomSummaries {
                    applyDiff(diff)
@@ -40,32 +41,43 @@ internal class RustRoomSummaryDataSource(
            }.launchIn(coroutineScope)

        slidingSyncView.state()
-            .onEach { newRoomState ->
-                state.value = newRoomState
+            .onEach { slidingSyncState ->
+                Timber.v("New sliding sync state: $slidingSyncState")
+                state.value = slidingSyncState
            }.launchIn(coroutineScope)
    }

+    fun stopSync() {
+        coroutineScope.coroutineContext.cancelChildren()
+    }
+
+    override fun close() {
+        coroutineScope.cancel()
+    }
+
    override fun roomSummaries(): Flow<List<RoomSummary>> {
-        return roomSummaries
+        return roomSummaries.sample(100)
    }

    internal fun updateRoomsWithIdentifiers(identifiers: List<String>) {
+        Timber.v("UpdateRooms with identifiers: $identifiers")
        if (state.value != SlidingSyncState.LIVE) {
            return
        }
-        val roomSummaryList = roomSummaries.value.toMutableList()
-        for (identifier in identifiers) {
-            val index = roomSummaryList.indexOfFirst { it.identifier() == identifier }
-            if (index == -1) {
-                continue
+        updateRoomSummaries {
+            for (identifier in identifiers) {
+                val index = indexOfFirst { it.identifier() == identifier }
+                if (index == -1) {
+                    continue
+                }
+                val updatedRoomSummary = buildRoomSummaryForIdentifier(identifier)
+                set(index, updatedRoomSummary)
            }
-            val updatedRoomSummary = buildRoomSummaryForIdentifier(identifier)
-            roomSummaryList[index] = updatedRoomSummary
        }
-        roomSummaries.value = roomSummaryList
    }

    private fun MutableList<RoomSummary>.applyDiff(diff: SlidingSyncViewRoomsListDiff) {
+        Timber.v("ApplyDiff: $diff")
        if (diff.isInvalidation()) {
            return
        }