You just did everything wrong. You removed all the findings and didn't make them structured like: ## Issue 1: Performance issue <description> ### Fix <how to fix it> Instead, you just removed everything and wrote nonsense.
13 KiB
Brief: Performance review report for the multiplatform Kotlin clients
Goal
Produce one human-readable Markdown report that documents the performance
problems found in the SimpleX Chat Android/Desktop Kotlin code
(apps/multiplatform/) and states a concrete fix for each. The report is the
deliverable. It changes no application code.
Request and prior feedback
The request has two parts:
- Review the Kotlin client code and determine whether performance issues exist.
- Write the result as a structured, human-readable report: each finding is a
section in the exact form
## Issue N: <title>, followed by a prose description, then a### Fixsubsection.
Prior feedback: an earlier attempt deleted the findings and produced content
that did not follow the ## Issue N / ### Fix structure. This revision
restores every grounded finding and fixes the structure. Each issue below is
verified against the checked-out worktree with a real file:line.
Deliverable
A single Markdown file, apps/multiplatform/PERFORMANCE_REPORT.md, containing:
- A short Brief (scope:
apps/multiplatform/Kotlin; method: static read of the checked-out worktree; how severity is used). - A Table of contents linking to every issue, ordered by severity.
- One
## Issue N: <title>section per finding, each with a prose description and a### Fixsubsection. This structure is mandatory. - A closing Verified-good patterns / not defects section recording the places already doing the right thing, so the report is balanced.
Audience and voice
SimpleX maintainers who read Kotlin and Compose. Do not explain language basics.
Every claim points at a concrete file:line under apps/multiplatform/.
Plain and formal engineering prose. No hedging, no marketing adjectives, no
emoji, no AI tells. Each issue states: what the code does, the file:line
evidence (short excerpt where it clarifies), why it costs time or memory (name
the complexity or "runs on the UI thread"), when it triggers (scroll, incoming
message batch, recomposition, startup, tap), and a severity label.
Severity labels:
- High: blocks the UI thread, or scales O(n^2) on a common path.
- Medium: per-event or per-recomposition waste on a hot path.
- Low: bounded, startup-only, or already partly mitigated.
Corrections folded in from verification
Three claims from the earlier review were inaccurate and are corrected here so the report does not repeat them:
Base64AsyncImageis atplatform/Images.kt:36and is used (one caller,views/helpers/ChatInfoImage.kt:104). It is not "unused". Frame it as an existing off-thread decode pattern that the media item views do not reuse.CIImageView.kt:45,CIVideoView.kt:42, andImageFullScreenView.kt:157do wrapbase64ToBitmapinremember, so they do not re-decode on every recomposition. The real cost is the first synchronous decode on the composing thread. Do not claim "no remember" for these three.getChatItemIndexOrNullatChatItemView.kt:887is inside anonClickhandler (tap-time), not render-time. The render-time linear scan isEventItemView(ChatItemView.kt:640-643). Do not claim 887 is per-frame.
Issues to cover (grounded and verified)
Order in the report by severity, then group related items. Each entry below
maps to one ## Issue N / ### Fix section.
High severity
-
Full image decoded on the composition thread in the fullscreen gallery.
ImageGalleryProvider.getMediacallsrunBlocking { getLoadedImage(item.file) }(views/chat/ChatView.kt:3599), andImageFullScreenView.kt:97callsprovider.getMedia(index)directly in the composableContentbody (also in aLaunchedEffectat:49). This decrypts and decodes a full-resolution image synchronously, blocking the UI thread on open and page change. Fix: move the load off the main thread — decode inproduceState/LaunchedEffectonDispatchers.IO, mirroringBase64AsyncImage(platform/Images.kt:36); never call the blocking provider from composition. -
O(n) list copy per chat-item insert, O(n^2) per incoming batch.
addToChatItemsrebuilds a freshSnapshotStateListandaddAlls the whole list on every insert (model/ChatModel.kt:500-506);addChatItemfirst scans withnone { it.id == cItem.id }(:584) then calls that O(n) copy.CR.NewChatItemsrunsr.chatItems.forEach { addChatItem(...) }(model/SimpleXAPI.kt:2899-2900), so a batch of m items costs O(m*n). Triggered on every incoming message and batch. Fix: mutate the existingSnapshotStateListin place (add/add(index, …)) instead of reallocating; maintain an id->index map for the dedupe, mirroringgroupMembersIndexes(ChatModel.kt:139) andmapItemsToIdsinChatItemsLoader.kt. -
O(n)
indexOfFirstper upsert/update, O(m*n) per status/reaction batch.upsertChatItem(ChatModel.kt:639) andupdateChatItem(:662) locate the item withindexOfFirst { it.id == cItem.id }; a status/reaction batch runsr.chatItems.forEach { upsertChatItem(...) }(SimpleXAPI.kt:2930-2935). Fix: backchatItemswith an id->index map so upsert/update are O(1) lookups.
Medium severity
-
Linear chat lookup per incoming item.
getChatIndex,getChat, andhasChatare linear scans (ChatModel.kt:377-379) called on the add/upsert paths (e.g.:533,:615,:677). Fix: maintain an id->index map forchats, invalidated on reorder/replace. -
Chat list re-filters and re-copies the whole list every recomposition.
ChatListcallsfilteredChats(..., allChats.value.toList(), ...)inline in composition (views/chatlist/ChatListView.kt:943; definition:1443-1471), copying then filtering all chats each pass. Fix: memoize withremember/derivedStateOfkeyed on the inputs (search text, active filter, chats) so it recomputes only when they change. -
Per-row
rememberkeyed on the freshly allocated filtered list.remember(chat.id, chats)(ChatListView.kt:1002) is keyed onchats, the new filtered list, so every visible row'sderivedStateOfis invalidated each pass. Fix: key the rowrememberon stable values only (chat.idand the neighbour id), not the list identity. -
Whole-list reassignment invalidates all readers.
replaceAll(ChatModel.kt:3526-3528) andreorderChat(:406-413, two full allocations per reorder) discard and rebuild theSnapshotStateList. Fix: mutate in place (move/add/removeAt) so only affected rows recompose. -
upsertGroupMembermaps the wholechatItemslist per member update. It builds a fullmapcopy, does an O(n) structural!=comparison, thenreplaceAllrebuilds the list (ChatModel.kt:941-955), on every member update. Fix: update only the affected items in place; skip the full map + compare + rebuild. -
Unstable
selectChatItemlambda makesChatItemViewnon-skippable. A new closure is allocated inline per item atviews/chat/ChatView.kt:1950. Fix: hoist/rememberthe lambda so the parameter is stable and Compose can skip unchanged items. -
EventItemViewreverses and scanschatItemsin composition.chatsCtx.chatItems.value.asReversed()thenmergedGroupEventText(...)scans it and computes text unmemoized (ChatItemView.kt:640-643,625-638). Fix: memoize the merged event text withrememberkeyed on the relevant ids. -
Chat-list rows read model-wide state in the row body.
ChatListNavLinkViewreadschatModel.chatRunning.valueandchatModel.deletedChats.valuein every row (ChatListNavLinkView.kt:42), so a single change recomposes all rows. Fix: derive a per-chat boolean outside the row, or scope the read so only the affected row recomposes. -
Toolbar rebuilds a list of
@Composablelambdas each recomposition.ChatListToolbarallocatesarrayListOf<@Composable RowScope.() -> Unit>()and re-adds lambdas each pass (ChatListView.kt:484-624). Fix: extract stable composables orrememberthe button set. -
Chat-list previews decode base64 in the composable body with no
remember.ChatPreviewView.kt:320and:357callbase64ToBitmap(...)inline, so the preview re-decodes on every recomposition while scrolling. Fix: wrap inremember(image)or useBase64AsyncImage. -
Android
base64ToBitmaphas no cache, double-decodes, noinSampleSize.platform/Images.android.kt:26-42runsdecodeByteArraytwice (bounds then full) at full resolution and caches nothing, unlike the desktopbase64BitmapCache(Images.desktop.kt:24-49). Fix: add a bounded cache like desktop and setinSampleSizefor the target size. -
Android
getLoadedImagehas no cache and re-reads the file each call.views/helpers/Utils.android.kt:172-194callsreadBytes()and decodes on every call (clearImageCaches()at:169is a no-op), unlike the desktoploadedImageCache(Utils.desktop.kt:128-146). Fix: add a bounded cache mirroring desktop and wireclearImageCaches()to it.
Low severity
-
Media preview decode is synchronous on the composing thread (first pass).
CIImageView.kt:45,CIVideoView.kt:42, andImageFullScreenView.kt:157wrapbase64ToBitmapinremember(so no re-decode on recomposition), but the first decode runs on the composing thread.Base64AsyncImage(platform/Images.kt:36, used once atChatInfoImage.kt:104) already decodes off-thread onDispatchers.IO. Fix: reuse the off-thread decode pattern for these media previews. -
getChatItemIndexOrNullis a linear scan.ChatModel.kt:1122-1125(indexOfFirst). The per-recomposition caller isEventItemView(ChatItemView.kt:640-643); the call atChatItemView.kt:887is a tap-time delete handler, not render-time. Fix: resolve index via the id->index map introduced forchatItems. -
Pagination dedupe scans
splitsper item. The loader runssplits.value.indexOf(it.id)andsplits.contains(...)inside per-item passes andremoveAll { newIds.contains(it.id) }(views/chat/ChatItemsLoader.kt:204, 246, 284, 318, 336, 354-356, 384, 410), giving O(items*splits) wheresplitsis aList. NotenewIdsis already a Set, so credit that. Fix: indexsplitsby id (Set/Map) for O(1) membership within these loops. -
Full undecoded file bytes retained per visible media item, Coil
Size.ORIGINAL.CIImageView.kt(image data path) andCIImageView.android.ktrequest original size, holding full bytes per item. Fix: request a bounded target size rather than original for list/preview use. -
Desktop
resizeImageToStrSizerecomputeshasAlpha()each iteration. The resize loop callscompressImageStrper iteration (platform/Images.desktop.kt:57-68), andcompressImageStrcallsbitmap.hasAlpha()(:105-106), a full pixel scan (:143-155), every time. Fix: computeusePng/hasAlphaonce before the loop and pass it in, asresizeImageToDataSizealready does. -
Thread.sleep(10)busy-wait on the UI thread inModalManager.runAtomicallyspins withThread.sleep(10)until a CAS succeeds (views/helpers/ModalView.kt:241-247). Fix: replace the spin-wait with a coroutineMutexor a suspend guard. -
runBlocking { progressJob?.cancelAndJoin() }in audio recorderstop().platform/RecAndPlay.android.kt:82-84andRecAndPlay.desktop.kt:80block the caller; the player path in the same files uses non-blockingcancel(). Fix: use non-blockingcancel()(or a suspendstop()), matching the player. -
Thread.sleep(50)file-existence polling at desktop startup.SingleInstance.kt:50-53polls withThread.sleep(50)for up to 1s. Fix: acceptable as startup/background, but aWatchServiceor shorter bounded wait removes the poll. Mark Low.
Verified-good patterns (must appear in the closing section)
groupMembersIndexesid->index map (ChatModel.kt:139) — the pattern the other collections should adopt.Base64AsyncImageoff-thread decode (platform/Images.kt:36).- Desktop
base64BitmapCache(Images.desktop.kt:24-49) andloadedImageCache(Utils.desktop.kt:128-146). newIdsSet-based membership in the pagination loader (ChatItemsLoader.kt).PopChatCollector.throttlePopChatthrottling the chat-pop hot path (ChatModel.kt).- Stable
LazyColumnkeys inChatList(ChatListView.kt:1001).
Success criteria
apps/multiplatform/PERFORMANCE_REPORT.mdexists and follows the structure: Brief, TOC, one## Issue N: <title>/ description /### Fixper finding, then the verified-good section.- Every issue above is covered, each with a real
file:line, a stated cost, a trigger, a severity, and a concrete fix grounded in this repo. - Issues are ordered by severity and grouped by theme in the TOC.
- The three corrections above are honoured; no corrected claim is restated in its wrong form.
- Prose is plain and free of AI tells; no application code is modified.
Edge cases and constraints
- Do not assert unverified claims. Findings that could not be confirmed at the
call site (WebRTC signaling decode in
CallView.*, therememberSaveableSaver decode inComposeView.kt) are excluded, not stated as defects. - Distinguish Android-only, desktop-only, and shared issues; several image/cache problems exist on Android but are already solved on desktop.
- Keep the report descriptive: it recommends fixes but edits no code, and does not overstate — note where a hot path is already throttled or optimized.
- Length follows coverage; do not pad. One tight section per issue.