From 8a057168ae3601af5c8933953f627b33bd24ad04 Mon Sep 17 00:00:00 2001 From: jeffrey-signal Date: Fri, 18 Jul 2025 16:36:17 -0400 Subject: [PATCH] Improve conversation scroll performance. Fixes a performance bottleneck in `ConversationFragment` caused by expensive calculations in `ConversationItemDecorations.hasHeader()`. This method is invoked in `RecyclerView.ItemDecoration.getItemOffsets()`, which runs on every layout pass and happens frequently during scrolling. The most expensive calculation in `hasHeader()` is `toEpochDay()`. That method calls `Long.toLocalDate()`, which clones a `TimeZone` object on each call. Upon opening one conversation (without scrolling), I observed that `toEpochDay()` was called over 1000 times in less than a second, rapidly allocating memory and causing garbage collection pressure that potentially leads to ANRs. We only need to calculate `hasHeader()` once for each conversation element, so caching the result of that method will eliminate the unnecessary calculations and improve the memory usage of `ConversationFragment`. --- .../v2/ConversationItemDecorations.kt | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationItemDecorations.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationItemDecorations.kt index e52fab04f8..802f185e96 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationItemDecorations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationItemDecorations.kt @@ -34,7 +34,7 @@ private typealias ConversationElement = MappingModel<*> * This is a converted and trimmed down version of [org.thoughtcrime.securesms.util.StickyHeaderDecoration]. */ class ConversationItemDecorations(hasWallpaper: Boolean = false, private val scheduleMessageMode: Boolean = false) : RecyclerView.ItemDecoration() { - + private val hasHeaderByPosition: MutableMap = mutableMapOf() private val headerCache: MutableMap = hashMapOf() private var unreadViewHolder: UnreadViewHolder? = null @@ -48,6 +48,7 @@ class ConversationItemDecorations(hasWallpaper: Boolean = false, private val sch set(value) { field = value updateUnreadState(value) + hasHeaderByPosition.clear() } var hasWallpaper: Boolean = hasWallpaper @@ -212,7 +213,21 @@ class ConversationItemDecorations(hasWallpaper: Boolean = false, private val sch (currentItems[bindingAdapterPosition] as? ConversationMessageElement)?.timestamp() == state.firstUnreadTimestamp } + /** + * Determines whether the item at [bindingAdapterPosition] should have a header. + */ private fun hasHeader(bindingAdapterPosition: Int): Boolean { + return hasHeaderByPosition.getOrPut( + key = bindingAdapterPosition, + defaultValue = { calculateHasHeader(bindingAdapterPosition) } + ) + } + + /** + * Determines whether the item at [bindingAdapterPosition] should have a header. Avoid using this method in performance critical areas, because it + * bypasses the caching mechanism used in [hasHeader]. + */ + private fun calculateHasHeader(bindingAdapterPosition: Int): Boolean { val model = if (bindingAdapterPosition in currentItems.indices) { currentItems[bindingAdapterPosition] } else { @@ -236,9 +251,13 @@ class ConversationItemDecorations(hasWallpaper: Boolean = false, private val sch return false } - return model.toEpochDay() != previousDay + val result = model.toEpochDay() != previousDay + return result } + /** + * Creates a header view for the provided [ConversationMessageElement] and caches it for future use. + */ private fun getHeader(parent: RecyclerView, model: ConversationMessageElement): DateHeaderViewHolder { val headerHolder: DateHeaderViewHolder = headerCache.getOrPut(model.toEpochDay()) { val view = LayoutInflater.from(parent.context).inflate(R.layout.conversation_item_header, parent, false)