RoomList: use same logic than Timeline for caching built items. (#1013)

* RoomList: use same logic than Timeline for caching built items. Extract into reusable components.

* RoomList: fix tests

* Fix `DiffCacheUpdater` docs

---------

Co-authored-by: ganfra <francoisg@element.io>
Co-authored-by: Jorge Martín <jorgem@element.io>
This commit is contained in:
ganfra 2023-08-01 10:53:41 +02:00 committed by GitHub
parent 89b1bba96e
commit e453b984ef
11 changed files with 373 additions and 142 deletions

View file

@ -1,56 +0,0 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.messages.impl.timeline.diff
import androidx.recyclerview.widget.ListUpdateCallback
import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.features.messages.impl.timeline.util.invalidateLast
import timber.log.Timber
internal class CacheInvalidator(private val itemStatesCache: MutableList<TimelineItem?>) :
ListUpdateCallback {
override fun onChanged(position: Int, count: Int, payload: Any?) {
Timber.d("onChanged(position= $position, count= $count)")
(position until position + count).forEach {
// Invalidate cache
itemStatesCache[it] = null
}
}
override fun onMoved(fromPosition: Int, toPosition: Int) {
Timber.d("onMoved(fromPosition= $fromPosition, toPosition= $toPosition)")
val model = itemStatesCache.removeAt(fromPosition)
itemStatesCache.add(toPosition, model)
}
override fun onInserted(position: Int, count: Int) {
Timber.d("onInserted(position= $position, count= $count)")
itemStatesCache.invalidateLast()
repeat(count) {
itemStatesCache.add(position, null)
}
}
override fun onRemoved(position: Int, count: Int) {
Timber.d("onRemoved(position= $position, count= $count)")
itemStatesCache.invalidateLast()
repeat(count) {
itemStatesCache.removeAt(position)
}
}
}

View file

@ -1,50 +0,0 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.messages.impl.timeline.diff
import androidx.recyclerview.widget.DiffUtil
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
internal class MatrixTimelineItemsDiffCallback(
private val oldList: List<MatrixTimelineItem>,
private val newList: List<MatrixTimelineItem>
) : DiffUtil.Callback() {
override fun getOldListSize(): Int {
return oldList.size
}
override fun getNewListSize(): Int {
return newList.size
}
override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
val oldItem = oldList.getOrNull(oldItemPosition)
val newItem = newList.getOrNull(newItemPosition)
return if (oldItem is MatrixTimelineItem.Event && newItem is MatrixTimelineItem.Event) {
oldItem.uniqueId == newItem.uniqueId
} else {
false
}
}
override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
val oldItem = oldList.getOrNull(oldItemPosition)
val newItem = newList.getOrNull(newItemPosition)
return oldItem == newItem
}
}

View file

@ -0,0 +1,63 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.messages.impl.timeline.diff
import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.libraries.androidutils.diff.DefaultDiffCacheInvalidator
import io.element.android.libraries.androidutils.diff.DiffCacheInvalidator
import io.element.android.libraries.androidutils.diff.MutableDiffCache
/**
* [DiffCacheInvalidator] implementation for [TimelineItem].
* It uses [DefaultDiffCacheInvalidator] and invalidate the cache around the updated item so that those items are computed again.
* This is needed because a timeline item is computed based on the previous and next items.
*/
internal class TimelineItemsCacheInvalidator : DiffCacheInvalidator<TimelineItem> {
private val delegate = DefaultDiffCacheInvalidator<TimelineItem>()
override fun onChanged(position: Int, count: Int, cache: MutableDiffCache<TimelineItem>) {
delegate.onChanged(position, count, cache)
}
override fun onMoved(fromPosition: Int, toPosition: Int, cache: MutableDiffCache<TimelineItem>) {
delegate.onMoved(fromPosition, toPosition, cache)
}
override fun onInserted(position: Int, count: Int, cache: MutableDiffCache<TimelineItem>) {
cache.invalidateAround(position)
delegate.onInserted(position, count, cache)
}
override fun onRemoved(position: Int, count: Int, cache: MutableDiffCache<TimelineItem>) {
cache.invalidateAround(position)
delegate.onRemoved(position, count, cache)
}
}
/**
* Invalidate the cache around the given position.
* It invalidates the previous and next items.
*/
private fun MutableDiffCache<*>.invalidateAround(position: Int) {
if (position > 0) {
set(position - 1, null)
}
if (position < indices().last) {
set(position + 1, null)
}
}

View file

@ -19,13 +19,13 @@ package io.element.android.features.messages.impl.timeline.factories
import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
import androidx.compose.runtime.collectAsState
import androidx.recyclerview.widget.DiffUtil
import io.element.android.features.messages.impl.timeline.diff.CacheInvalidator
import io.element.android.features.messages.impl.timeline.diff.MatrixTimelineItemsDiffCallback
import io.element.android.features.messages.impl.timeline.diff.TimelineItemsCacheInvalidator
import io.element.android.features.messages.impl.timeline.factories.event.TimelineItemEventFactory
import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory
import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.libraries.androidutils.diff.DiffCacheUpdater
import io.element.android.libraries.androidutils.diff.MutableListDiffCache
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
import kotlinx.collections.immutable.ImmutableList
@ -35,9 +35,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import timber.log.Timber
import javax.inject.Inject
import kotlin.system.measureTimeMillis
class TimelineItemsFactory @Inject constructor(
private val dispatchers: CoroutineDispatchers,
@ -46,13 +44,20 @@ class TimelineItemsFactory @Inject constructor(
private val timelineItemGrouper: TimelineItemGrouper,
) {
private val timelineItems = MutableStateFlow(persistentListOf<TimelineItem>())
private val timelineItemsCache = arrayListOf<TimelineItem?>()
// Items from rust sdk, used for diffing
private var matrixTimelineItems: List<MatrixTimelineItem> = emptyList()
private val lock = Mutex()
private val cacheInvalidator = CacheInvalidator(timelineItemsCache)
private val diffCache = MutableListDiffCache<TimelineItem>()
private val diffCacheUpdater = DiffCacheUpdater<MatrixTimelineItem, TimelineItem>(
diffCache = diffCache,
detectMoves = false,
cacheInvalidator = TimelineItemsCacheInvalidator()
) { old, new ->
if (old is MatrixTimelineItem.Event && new is MatrixTimelineItem.Event) {
old.uniqueId == new.uniqueId
} else {
false
}
}
@Composable
fun collectItemsAsState(): State<ImmutableList<TimelineItem>> {
@ -63,15 +68,15 @@ class TimelineItemsFactory @Inject constructor(
timelineItems: List<MatrixTimelineItem>,
) = withContext(dispatchers.computation) {
lock.withLock {
calculateAndApplyDiff(timelineItems)
diffCacheUpdater.updateWith(timelineItems)
buildAndEmitTimelineItemStates(timelineItems)
}
}
private suspend fun buildAndEmitTimelineItemStates(timelineItems: List<MatrixTimelineItem>) {
val newTimelineItemStates = ArrayList<TimelineItem>()
for (index in timelineItemsCache.indices.reversed()) {
val cacheItem = timelineItemsCache[index]
for (index in diffCache.indices().reversed()) {
val cacheItem = diffCache.get(index)
if (cacheItem == null) {
buildAndCacheItem(timelineItems, index)?.also { timelineItemState ->
newTimelineItemStates.add(timelineItemState)
@ -84,20 +89,6 @@ class TimelineItemsFactory @Inject constructor(
this.timelineItems.emit(result)
}
private fun calculateAndApplyDiff(newTimelineItems: List<MatrixTimelineItem>) {
val timeToDiff = measureTimeMillis {
val diffCallback =
MatrixTimelineItemsDiffCallback(
oldList = matrixTimelineItems,
newList = newTimelineItems
)
val diffResult = DiffUtil.calculateDiff(diffCallback, false)
matrixTimelineItems = newTimelineItems
diffResult.dispatchUpdatesTo(cacheInvalidator)
}
Timber.v("Time to apply diff on new list of ${newTimelineItems.size} items: $timeToDiff ms")
}
private fun buildAndCacheItem(
timelineItems: List<MatrixTimelineItem>,
index: Int
@ -108,7 +99,7 @@ class TimelineItemsFactory @Inject constructor(
is MatrixTimelineItem.Virtual -> virtualItemFactory.create(currentTimelineItem)
MatrixTimelineItem.Other -> null
}
timelineItemsCache[index] = timelineItemState
diffCache[index] = timelineItemState
return timelineItemState
}
}