perf(mailbox): page the unified "All inboxes" list (#124)
The unified inbox query (WHERE folder = ?, no accountId) has no folder-leading index, so it scans in timestamp order and materializes the whole unified inbox (~4k rows at a 20k cache) into memory on every emission. Apply Paging 3 to the unified browse path so query, mapping, and recomposition cost scale with the visible window, not the total cache. - MessageDao.pagingUnifiedFolderSummaries: a PagingSource over the folder's synced rows (inInbox = 1); unified search keeps the whole-folder query so it can still surface transient server-search hits. - MailRepository.pagedUnifiedFolderMessages: a Pager (pageSize 40, initialLoad 120, no placeholders) mapping summaries to domain. - MailboxViewModel.pagedMessages: paged while browsing the unified inbox, else empty; the messages list flow stays empty in that state so the whole cache is never materialized. Selection captures each row's accountId at tap time, so "Move" still resolves the selection's account without an in-memory list. - MailboxScreen renders the unified browse list via collectAsLazyPagingItems; per-account and search views render the flat list unchanged (issue #86 stays flat). Profiling (docs/perf/issue-124-unified-inbox-paging.md) on an api29 emulator: current whole-inbox first-emit ~24.6 ms at a 20k cache vs. the paged first page ~6.8 ms and flat regardless of cache size (~3.6x). EXPLAIN QUERY PLAN shows the paged query still stops early on the existing timestamp index, so no (folder, ...) index and no schema migration are added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Claude Fable 5
@@ -200,9 +200,14 @@ dependencies {
|
||||
|
||||
implementation(libs.androidx.room.runtime)
|
||||
implementation(libs.androidx.room.ktx)
|
||||
implementation(libs.androidx.room.paging)
|
||||
ksp(libs.androidx.room.compiler)
|
||||
implementation(libs.sqlcipher.android)
|
||||
|
||||
// Paging 3 — the unified inbox list is paged so its cost scales with the screen (issue #124).
|
||||
implementation(libs.androidx.paging.runtime)
|
||||
implementation(libs.androidx.paging.compose)
|
||||
|
||||
// Raise kotlinx-serialization to the version Room's schema-bundle serializers were compiled
|
||||
// against (see libs.versions.toml). AGP 9 consistent resolution shares it with the androidTest
|
||||
// classpath so MigrationTestHelper can parse the exported schema JSON.
|
||||
@@ -214,6 +219,8 @@ dependencies {
|
||||
testImplementation(libs.turbine)
|
||||
testImplementation(libs.mockk)
|
||||
testImplementation(libs.greenmail)
|
||||
// asSnapshot() drives a PagingData flow to a concrete list in JVM unit tests (issue #124).
|
||||
testImplementation(libs.androidx.paging.testing)
|
||||
// The real org.json for unit tests (android.jar ships a stubbed, no-op version).
|
||||
testImplementation("org.json:json:20231013")
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui
|
||||
|
||||
import androidx.paging.PagingData
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
@@ -92,6 +93,9 @@ class FakeMailRepository(
|
||||
override fun observeUnifiedFolderMessages(folder: String): Flow<List<Message>> =
|
||||
flowOf(messages.filter { it.folder == folder })
|
||||
|
||||
override fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>> =
|
||||
flowOf(PagingData.from(messages.filter { it.folder == folder && it.inInbox }))
|
||||
|
||||
override fun observeFolders(accountId: String): Flow<List<Folder>> = flowOf(
|
||||
folders.filter {
|
||||
it.accountId ==
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local.dao
|
||||
|
||||
import androidx.paging.PagingSource
|
||||
import androidx.room.Dao
|
||||
import androidx.room.Insert
|
||||
import androidx.room.OnConflictStrategy
|
||||
@@ -54,6 +55,24 @@ interface MessageDao {
|
||||
)
|
||||
fun observeUnifiedFolderSummaries(folder: String): Flow<List<MessageSummary>>
|
||||
|
||||
/**
|
||||
* Paged unified-inbox projection: folder-synced rows of [folder] across every account,
|
||||
* newest-first, as a Paging 3 [PagingSource] (issue #124). Unlike [observeUnifiedFolderSummaries]
|
||||
* — which materializes the *entire* unified inbox (~thousands of rows) on every emission — Room
|
||||
* loads only the requested window (LIMIT/OFFSET), so the mailbox list's query, mapping, and
|
||||
* recomposition cost scale with what's on screen, not the whole cache. Filters `inInbox = 1`
|
||||
* because the paged browse list shows only synced rows; unified *search* (which must also surface
|
||||
* transient `inInbox = 0` hits) stays on [observeUnifiedFolderSummaries]. Profiling (see
|
||||
* `docs/perf/issue-124-unified-inbox-paging.md`) showed the first page loads flat regardless of
|
||||
* total cache size on the existing indices, so no `(folder, …)` index / schema migration is added.
|
||||
*/
|
||||
@Query(
|
||||
"SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " +
|
||||
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages " +
|
||||
"WHERE folder = :folder AND inInbox = 1 ORDER BY timestampMillis DESC",
|
||||
)
|
||||
fun pagingUnifiedFolderSummaries(folder: String): PagingSource<Int, MessageSummary>
|
||||
|
||||
/**
|
||||
* Live per-(account, folder) unread counts for the drawer's folder badges and the bold styling of
|
||||
* accounts with unread mail. Counts only folder-synced rows (`inInbox = 1`), so transient
|
||||
|
||||
@@ -3,6 +3,10 @@ package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import android.net.Uri
|
||||
import androidx.paging.Pager
|
||||
import androidx.paging.PagingConfig
|
||||
import androidx.paging.PagingData
|
||||
import androidx.paging.map
|
||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||
import jakarta.mail.Flags
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
@@ -66,6 +70,19 @@ class MailRepositoryImpl @Inject constructor(
|
||||
override fun observeUnifiedFolderMessages(folder: String): Flow<List<Message>> =
|
||||
messageDao.observeUnifiedFolderSummaries(folder).map { rows -> rows.map { it.toDomain() } }
|
||||
|
||||
override fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>> = Pager(
|
||||
config = PagingConfig(
|
||||
// A page comfortably exceeds a screenful so scrolling rarely waits on a load; loading
|
||||
// three pages up front fills the first viewport without a visible gap. Placeholders
|
||||
// are off: the row height varies (snippet/account label), so a fixed-height placeholder
|
||||
// would jump, and the list never needs a scrollbar sized to the full (uncounted) inbox.
|
||||
pageSize = MAILBOX_PAGE_SIZE,
|
||||
initialLoadSize = MAILBOX_PAGE_SIZE * 3,
|
||||
enablePlaceholders = false,
|
||||
),
|
||||
pagingSourceFactory = { messageDao.pagingUnifiedFolderSummaries(folder) },
|
||||
).flow.map { page -> page.map { it.toDomain() } }
|
||||
|
||||
override fun observeFolders(accountId: String): Flow<List<Folder>> =
|
||||
folderDao.observeForAccount(accountId).map { rows ->
|
||||
rows.map { it.toDomain() }
|
||||
@@ -380,5 +397,8 @@ class MailRepositoryImpl @Inject constructor(
|
||||
|
||||
private const val SEARCH_LIMIT = 50
|
||||
|
||||
/** Rows per page for the unified inbox (issue #124) — a page is a few screenfuls of message rows. */
|
||||
private const val MAILBOX_PAGE_SIZE = 40
|
||||
|
||||
/** Message id is "<accountId>:<uid>"; the uid is the trailing segment. */
|
||||
private fun uidOf(id: String): String = id.substringAfterLast(':')
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.domain.repository
|
||||
|
||||
import androidx.paging.PagingData
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import org.libremail.domain.model.Attachment
|
||||
import org.libremail.domain.model.Draft
|
||||
@@ -27,6 +28,14 @@ interface MailRepository {
|
||||
/** Like [observeFolderMessages] but for [folder] across every account (the unified inbox). */
|
||||
fun observeUnifiedFolderMessages(folder: String): Flow<List<Message>>
|
||||
|
||||
/**
|
||||
* The unified inbox as a [PagingData] stream so the list's query, mapping, and recomposition cost
|
||||
* scale with the visible window rather than the whole cache (issue #124). Emits only folder-synced
|
||||
* rows of [folder] across every account, newest-first — unified *search* still uses
|
||||
* [observeUnifiedFolderMessages]. Callers must `cachedIn` a scope before collecting.
|
||||
*/
|
||||
fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>>
|
||||
|
||||
/** The account's cached IMAP folders for the navigation drawer. */
|
||||
fun observeFolders(accountId: String): Flow<List<Folder>>
|
||||
|
||||
|
||||
@@ -80,6 +80,9 @@ import androidx.compose.ui.text.style.TextOverflow
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.hilt.navigation.compose.hiltViewModel
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import androidx.paging.LoadState
|
||||
import androidx.paging.compose.collectAsLazyPagingItems
|
||||
import androidx.paging.compose.itemKey
|
||||
import kotlinx.coroutines.launch
|
||||
import org.libremail.R
|
||||
import org.libremail.domain.model.Account
|
||||
@@ -102,6 +105,8 @@ fun MailboxScreen(
|
||||
viewModel: MailboxViewModel = hiltViewModel(),
|
||||
) {
|
||||
val messages by viewModel.messages.collectAsStateWithLifecycle()
|
||||
// The unified "All inboxes" browse list is paged (issue #124); per-account/search render [messages].
|
||||
val pagedMessages = viewModel.pagedMessages.collectAsLazyPagingItems()
|
||||
val accounts by viewModel.accounts.collectAsStateWithLifecycle()
|
||||
val selectedAccountId by viewModel.selectedAccountId.collectAsStateWithLifecycle()
|
||||
val selectedFolder by viewModel.selectedFolder.collectAsStateWithLifecycle()
|
||||
@@ -123,6 +128,10 @@ fun MailboxScreen(
|
||||
val moveTargetFolders by viewModel.moveTargetFolders.collectAsStateWithLifecycle()
|
||||
val actionInProgress by viewModel.actionInProgress.collectAsStateWithLifecycle()
|
||||
val selectionMode = selectedIds.isNotEmpty()
|
||||
// The unified inbox (no account filter, not searching) renders the paged list; a concrete account
|
||||
// or an active search renders the flat [messages] list. Hoisted here so the selection bar's
|
||||
// "Select all" can read the right source (issue #124).
|
||||
val showPaged = selectedAccountId == null && searchQuery.isBlank()
|
||||
var showMovePicker by remember { mutableStateOf(false) }
|
||||
val snackbarHostState = remember { SnackbarHostState() }
|
||||
val drawerState = rememberDrawerState(DrawerValue.Closed)
|
||||
@@ -187,7 +196,13 @@ fun MailboxScreen(
|
||||
onDelete = viewModel::requestDelete,
|
||||
onSpam = viewModel::requestSpam,
|
||||
onMove = { showMovePicker = true },
|
||||
onSelectAll = viewModel::selectAll,
|
||||
onSelectAll = {
|
||||
// "Select all" acts on what's shown: the loaded paged window for the unified
|
||||
// inbox, or the whole flat list for a per-account/search view.
|
||||
viewModel.selectAll(
|
||||
if (showPaged) pagedMessages.itemSnapshotList.items else messages,
|
||||
)
|
||||
},
|
||||
onReply = { viewModel.reply(ReplyMode.REPLY) },
|
||||
onReplyAll = viewModel::requestReplyAll,
|
||||
onForward = { viewModel.reply(ReplyMode.FORWARD) },
|
||||
@@ -278,7 +293,42 @@ fun MailboxScreen(
|
||||
modifier = Modifier.fillMaxSize(),
|
||||
) {
|
||||
LazyColumn(modifier = Modifier.fillMaxSize()) {
|
||||
if (messages.isEmpty()) {
|
||||
if (showPaged) {
|
||||
if (pagedMessages.itemCount == 0) {
|
||||
item {
|
||||
// Hold the empty state back until the first page settles so
|
||||
// it doesn't flash before rows arrive (search never lands here).
|
||||
if (pagedMessages.loadState.refresh !is LoadState.Loading) {
|
||||
NoMessagesState(Modifier.fillParentMaxSize())
|
||||
}
|
||||
}
|
||||
} else {
|
||||
items(
|
||||
count = pagedMessages.itemCount,
|
||||
key = pagedMessages.itemKey { it.id },
|
||||
) { index ->
|
||||
val message = pagedMessages[index] ?: return@items
|
||||
val accountLabel =
|
||||
if (showAccount) accountsById[message.accountId]?.email else null
|
||||
MessageRow(
|
||||
message = message,
|
||||
accountLabel = accountLabel,
|
||||
selected = message.id in selectedIds,
|
||||
onClick = {
|
||||
if (selectionMode) {
|
||||
viewModel.toggleSelection(message.id, message.accountId)
|
||||
} else {
|
||||
onOpenMessage(message.id)
|
||||
}
|
||||
},
|
||||
onLongClick = {
|
||||
viewModel.startSelection(message.id, message.accountId)
|
||||
},
|
||||
)
|
||||
HorizontalDivider()
|
||||
}
|
||||
}
|
||||
} else if (messages.isEmpty()) {
|
||||
item {
|
||||
if (searchActive && searchQuery.isNotBlank()) {
|
||||
NoResultsState(Modifier.fillParentMaxSize())
|
||||
@@ -296,12 +346,12 @@ fun MailboxScreen(
|
||||
selected = message.id in selectedIds,
|
||||
onClick = {
|
||||
if (selectionMode) {
|
||||
viewModel.toggleSelection(message.id)
|
||||
viewModel.toggleSelection(message.id, message.accountId)
|
||||
} else {
|
||||
onOpenMessage(message.id)
|
||||
}
|
||||
},
|
||||
onLongClick = { viewModel.startSelection(message.id) },
|
||||
onLongClick = { viewModel.startSelection(message.id, message.accountId) },
|
||||
)
|
||||
HorizontalDivider()
|
||||
}
|
||||
|
||||
@@ -4,10 +4,13 @@ package org.libremail.ui.mailbox
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.paging.PagingData
|
||||
import androidx.paging.cachedIn
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.FlowPreview
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
@@ -116,27 +119,62 @@ class MailboxViewModel @Inject constructor(
|
||||
private val _searchQuery = MutableStateFlow("")
|
||||
val searchQuery: StateFlow<String> = _searchQuery.asStateFlow()
|
||||
|
||||
/**
|
||||
* The list-rendered messages: a per-account folder (SQL-scoped and already flat, issue #86) or —
|
||||
* for the unified inbox — *only* an active search's matches across accounts. The unified **browse**
|
||||
* list is paged instead (see [pagedMessages], issue #124), so this flow stays empty while browsing
|
||||
* the unified inbox and the whole cache is never pulled into memory on each write.
|
||||
*/
|
||||
val messages: StateFlow<List<Message>> =
|
||||
combine(_selectedAccountId, _selectedFolder) { accountId, folder -> accountId to folder }
|
||||
.distinctUntilChanged()
|
||||
.flatMapLatest { (accountId, folder) ->
|
||||
// Scope the query in SQL to the viewed account+folder (or [folder] across accounts for
|
||||
// the unified inbox) so Room only re-queries/re-emits when those rows change, and the
|
||||
// cost scales with the folder, not the whole cache (issue #86).
|
||||
val scoped = if (accountId == null) {
|
||||
mailRepository.observeUnifiedFolderMessages(folder)
|
||||
} else {
|
||||
mailRepository.observeFolderMessages(accountId, folder)
|
||||
}
|
||||
// The only remaining client-side pass distinguishes the normal list (synced rows) from
|
||||
// an active search (any matching row, including transient server-search hits) — a match
|
||||
// over the small folder-scoped set, never the whole cache.
|
||||
combine(scoped, _searchQuery) { rows, query ->
|
||||
if (accountId != null) {
|
||||
// Per-account+folder: the SQL-scoped, already-flat list. The one client-side pass
|
||||
// distinguishes the normal list (synced rows) from an active search (any matching
|
||||
// row, including transient server-search hits) over the small folder-scoped set.
|
||||
combine(mailRepository.observeFolderMessages(accountId, folder), _searchQuery) { rows, query ->
|
||||
val q = query.trim()
|
||||
rows.filter { if (q.isEmpty()) it.inInbox else it.matchesSearch(q) }
|
||||
}
|
||||
} else {
|
||||
// Unified inbox: browse is paged via [pagedMessages]; this backs only an active
|
||||
// search over the folder's rows across accounts (bounded by the per-account search
|
||||
// limit), staying empty while browsing so the whole inbox is never materialized.
|
||||
_searchQuery.flatMapLatest { query ->
|
||||
val q = query.trim()
|
||||
if (q.isEmpty()) {
|
||||
flowOf(emptyList())
|
||||
} else {
|
||||
mailRepository.observeUnifiedFolderMessages(folder)
|
||||
.map { rows -> rows.filter { it.matchesSearch(q) } }
|
||||
}
|
||||
}
|
||||
}
|
||||
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList())
|
||||
|
||||
/**
|
||||
* The unified "All inboxes" browse list as a paged stream (issue #124): folder-synced rows across
|
||||
* every account, newest-first, loaded a window at a time so query/mapping/recomposition cost scale
|
||||
* with the screen, not the total cache. Emits empty paging data whenever a concrete account is
|
||||
* selected or a search is active (those render from [messages]).
|
||||
*/
|
||||
val pagedMessages: Flow<PagingData<Message>> =
|
||||
combine(
|
||||
_selectedAccountId,
|
||||
_selectedFolder,
|
||||
_searchQuery.map { it.isBlank() }.distinctUntilChanged(),
|
||||
) { accountId, folder, browsing -> Triple(accountId, folder, browsing) }
|
||||
.distinctUntilChanged()
|
||||
.flatMapLatest { (accountId, folder, browsing) ->
|
||||
if (accountId == null && browsing) {
|
||||
mailRepository.pagedUnifiedFolderMessages(folder)
|
||||
} else {
|
||||
flowOf(PagingData.empty())
|
||||
}
|
||||
}
|
||||
.cachedIn(viewModelScope)
|
||||
|
||||
val draftCount: StateFlow<Int> = mailRepository.observeDrafts()
|
||||
.map { it.size }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), 0)
|
||||
@@ -156,6 +194,11 @@ class MailboxViewModel @Inject constructor(
|
||||
private val _selectedIds = MutableStateFlow<Set<String>>(emptySet())
|
||||
val selectedIds: StateFlow<Set<String>> = _selectedIds.asStateFlow()
|
||||
|
||||
// Account id per selected message id, captured at selection time. The unified inbox is paged
|
||||
// (issue #124), so the full list isn't held in memory; this lets [selectionAccountId] decide
|
||||
// whether a selection sits within a single account (→ Move offered) without materializing it.
|
||||
private val selectionAccounts = MutableStateFlow<Map<String, String>>(emptyMap())
|
||||
|
||||
private val _pendingConfirm = MutableStateFlow<PendingAction?>(null)
|
||||
val pendingConfirm: StateFlow<PendingAction?> = _pendingConfirm.asStateFlow()
|
||||
|
||||
@@ -173,9 +216,8 @@ class MailboxViewModel @Inject constructor(
|
||||
|
||||
/** The single account every selected message belongs to, or null if the selection spans accounts. */
|
||||
private val selectionAccountId: StateFlow<String?> =
|
||||
combine(_selectedIds, messages) { ids, msgs ->
|
||||
msgs.filter { it.id in ids }.map { it.accountId }.distinct().singleOrNull()
|
||||
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null)
|
||||
selectionAccounts.map { it.values.distinct().singleOrNull() }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null)
|
||||
|
||||
/** Whether "Move" is offered: only when the whole selection sits in a single account's folder tree. */
|
||||
val canMove: StateFlow<Boolean> = selectionAccountId
|
||||
@@ -187,20 +229,30 @@ class MailboxViewModel @Inject constructor(
|
||||
.flatMapLatest { acct -> if (acct == null) flowOf(emptyList()) else mailRepository.observeFolders(acct) }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList())
|
||||
|
||||
fun startSelection(id: String) {
|
||||
fun startSelection(id: String, accountId: String) {
|
||||
_selectedIds.value = setOf(id)
|
||||
selectionAccounts.value = mapOf(id to accountId)
|
||||
}
|
||||
|
||||
fun toggleSelection(id: String) {
|
||||
_selectedIds.value = _selectedIds.value.let { if (id in it) it - id else it + id }
|
||||
fun toggleSelection(id: String, accountId: String) {
|
||||
if (id in _selectedIds.value) {
|
||||
_selectedIds.value = _selectedIds.value - id
|
||||
selectionAccounts.value = selectionAccounts.value - id
|
||||
} else {
|
||||
_selectedIds.value = _selectedIds.value + id
|
||||
selectionAccounts.value = selectionAccounts.value + (id to accountId)
|
||||
}
|
||||
}
|
||||
|
||||
fun clearSelection() {
|
||||
_selectedIds.value = emptySet()
|
||||
selectionAccounts.value = emptyMap()
|
||||
}
|
||||
|
||||
fun selectAll() {
|
||||
_selectedIds.value = messages.value.map { it.id }.toSet()
|
||||
/** Selects everything currently shown; [items] is the visible list (paged snapshot or list). */
|
||||
fun selectAll(items: List<Message>) {
|
||||
_selectedIds.value = items.map { it.id }.toSet()
|
||||
selectionAccounts.value = items.associate { it.id to it.accountId }
|
||||
}
|
||||
|
||||
fun archiveSelected() = runOnSelection { mailRepository.archive(it) }
|
||||
|
||||
@@ -2,6 +2,9 @@
|
||||
package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import androidx.paging.PagingSource
|
||||
import androidx.paging.PagingState
|
||||
import androidx.paging.testing.asSnapshot
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.Runs
|
||||
import io.mockk.coEvery
|
||||
@@ -105,6 +108,20 @@ class MailRepositoryImplTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `pagedUnifiedFolderMessages maps the paged summaries to domain messages`() = runTest {
|
||||
every { messageDao.pagingUnifiedFolderSummaries("INBOX") } returns FakeSummaryPagingSource(
|
||||
listOf(messageSummary("1", "INBOX"), messageSummary("2", "INBOX", accountId = "acct2")),
|
||||
)
|
||||
|
||||
val items = repository.pagedUnifiedFolderMessages("INBOX").asSnapshot()
|
||||
|
||||
assertEquals(listOf("1", "2"), items.map { it.id })
|
||||
assertEquals("Ada", items.first().sender)
|
||||
// The list projection never carries a body — the reader loads it on demand (see MessageSummary).
|
||||
assertEquals("", items.first().body)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `observeFolders maps cached folders with their roles and server special-use flag`() = runTest {
|
||||
every { folderDao.observeForAccount("acct") } returns flowOf(
|
||||
@@ -604,4 +621,12 @@ class MailRepositoryImplTest {
|
||||
secret = "secret",
|
||||
useXoauth2 = false,
|
||||
)
|
||||
|
||||
/** Serves a fixed set of summaries as a single page, standing in for Room's generated source. */
|
||||
private class FakeSummaryPagingSource(private val rows: List<MessageSummary>) :
|
||||
PagingSource<Int, MessageSummary>() {
|
||||
override fun getRefreshKey(state: PagingState<Int, MessageSummary>): Int? = null
|
||||
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, MessageSummary> =
|
||||
LoadResult.Page(data = rows, prevKey = null, nextKey = null)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
package org.libremail.ui.mailbox
|
||||
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.paging.PagingData
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
@@ -53,7 +54,7 @@ class MailboxViewModelTest {
|
||||
private val bob = account("imap:b", "bob@example.org")
|
||||
|
||||
@Test
|
||||
fun `default view shows only inbox messages across all accounts`() = runTest(testDispatcher) {
|
||||
fun `unified browse keeps the whole inbox out of the in-memory list flow`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(
|
||||
accounts = listOf(alice, bob),
|
||||
messages = listOf(
|
||||
@@ -66,7 +67,24 @@ class MailboxViewModelTest {
|
||||
|
||||
assertEquals("INBOX", vm.selectedFolder.value)
|
||||
assertNull(vm.selectedAccountId.value)
|
||||
assertEquals(setOf("imap:a:INBOX:1", "imap:b:INBOX:1"), vm.messages.value.map { it.id }.toSet())
|
||||
// Unified browse is paged (issue #124): the flat list flow stays empty so the whole unified
|
||||
// inbox is never materialized on every write. The paged rows themselves are covered by
|
||||
// MailRepositoryImplTest's pagedUnifiedFolderMessages test and the MailboxScreen UI test.
|
||||
assertTrue(vm.messages.value.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `selecting a concrete account renders the flat scoped list`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(
|
||||
accounts = listOf(alice),
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX"), msg("imap:a:Archive:1", "imap:a", "Archive")),
|
||||
)
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
|
||||
vm.selectAccount("imap:a")
|
||||
|
||||
// A concrete account uses the SQL-scoped, already-flat list (issue #86), not the paged path.
|
||||
assertEquals(listOf("imap:a:INBOX:1"), vm.messages.value.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -200,11 +218,11 @@ class MailboxViewModelTest {
|
||||
fun `toggle adds then removes a message from the selection`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(accounts = listOf(alice), messages = emptyList())
|
||||
|
||||
vm.startSelection("a")
|
||||
vm.startSelection("a", "acct")
|
||||
assertEquals(setOf("a"), vm.selectedIds.value)
|
||||
vm.toggleSelection("b")
|
||||
vm.toggleSelection("b", "acct")
|
||||
assertEquals(setOf("a", "b"), vm.selectedIds.value)
|
||||
vm.toggleSelection("a")
|
||||
vm.toggleSelection("a", "acct")
|
||||
assertEquals(setOf("b"), vm.selectedIds.value)
|
||||
vm.clearSelection()
|
||||
assertTrue(vm.selectedIds.value.isEmpty())
|
||||
@@ -212,13 +230,11 @@ class MailboxViewModelTest {
|
||||
|
||||
@Test
|
||||
fun `selectAll selects every visible message`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(
|
||||
accounts = listOf(alice),
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX"), msg("imap:a:INBOX:2", "imap:a", "INBOX")),
|
||||
)
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
val shown = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX"), msg("imap:a:INBOX:2", "imap:a", "INBOX"))
|
||||
val vm = createViewModel(accounts = listOf(alice), messages = shown)
|
||||
|
||||
vm.selectAll()
|
||||
// The screen passes what's shown (paged snapshot or flat list); the VM records their ids.
|
||||
vm.selectAll(shown)
|
||||
|
||||
assertEquals(setOf("imap:a:INBOX:1", "imap:a:INBOX:2"), vm.selectedIds.value)
|
||||
}
|
||||
@@ -234,7 +250,7 @@ class MailboxViewModelTest {
|
||||
)
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.requestDelete()
|
||||
val pending = vm.pendingConfirm.value
|
||||
assertTrue(pending is PendingAction.Delete && !pending.permanent)
|
||||
@@ -265,7 +281,7 @@ class MailboxViewModelTest {
|
||||
backgroundScope.launch { vm.currentFolderRole.collect {} }
|
||||
vm.selectFolder("imap:a", "Spam")
|
||||
|
||||
vm.startSelection("imap:a:Spam:1")
|
||||
vm.startSelection("imap:a:Spam:1", "imap:a")
|
||||
vm.requestDelete()
|
||||
val pending = vm.pendingConfirm.value
|
||||
assertTrue(pending is PendingAction.Delete && pending.permanent)
|
||||
@@ -285,7 +301,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.requestSpam()
|
||||
assertTrue(vm.pendingConfirm.value is PendingAction.Spam)
|
||||
coVerify(exactly = 0) { repo.reportSpam(any()) }
|
||||
@@ -304,7 +320,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.reply(ReplyMode.FORWARD)
|
||||
|
||||
assertEquals(MailboxEvent.OpenCompose("draft1"), vm.events.first())
|
||||
@@ -321,7 +337,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.archiveSelected()
|
||||
|
||||
coVerify { repo.archive(listOf("imap:a:INBOX:1")) }
|
||||
@@ -338,7 +354,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.reply(ReplyMode.REPLY)
|
||||
|
||||
assertEquals(MailboxEvent.OpenCompose("d2"), vm.events.first())
|
||||
@@ -352,7 +368,7 @@ class MailboxViewModelTest {
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX")),
|
||||
repo = repo,
|
||||
)
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
|
||||
vm.requestSpam()
|
||||
vm.dismissConfirm()
|
||||
@@ -371,10 +387,10 @@ class MailboxViewModelTest {
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
backgroundScope.launch { vm.canMove.collect {} }
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
assertEquals(true, vm.canMove.value)
|
||||
|
||||
vm.toggleSelection("imap:b:INBOX:1")
|
||||
vm.toggleSelection("imap:b:INBOX:1", "imap:b")
|
||||
assertEquals(false, vm.canMove.value)
|
||||
}
|
||||
|
||||
@@ -388,7 +404,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.moveSelected("Receipts")
|
||||
|
||||
coVerify { repo.moveToFolder(listOf("imap:a:INBOX:1"), "Receipts") }
|
||||
@@ -404,7 +420,7 @@ class MailboxViewModelTest {
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX")),
|
||||
repo = repo,
|
||||
)
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
|
||||
vm.requestReplyAll()
|
||||
assertTrue(vm.pendingConfirm.value is PendingAction.ReplyAll)
|
||||
@@ -493,6 +509,12 @@ class MailboxViewModelTest {
|
||||
val folder = firstArg<String>()
|
||||
MutableStateFlow(messages.filter { it.folder == folder })
|
||||
}
|
||||
// The unified browse list is paged (issue #124); mirror the DAO's inInbox-scoped, folder-scoped
|
||||
// projection as a single static page.
|
||||
every { repo.pagedUnifiedFolderMessages(any()) } answers {
|
||||
val folder = firstArg<String>()
|
||||
flowOf(PagingData.from(messages.filter { it.folder == folder && it.inInbox }))
|
||||
}
|
||||
every { repo.observeDrafts() } returns flowOf(emptyList())
|
||||
every { repo.observeOutbox() } returns flowOf(emptyList())
|
||||
every { repo.observeUnreadCounts() } returns MutableStateFlow(unreadCounts)
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
<!-- SPDX-License-Identifier: GPL-3.0-or-later -->
|
||||
# Issue #124 — paging the unified "All inboxes" view
|
||||
|
||||
Follow-up to #86. PR #123 made the per-account+folder list flat by scoping the query in SQL, but the
|
||||
unified inbox (`WHERE folder = ?`, no `accountId`) has no `folder`-leading index, so it still **scans**
|
||||
in timestamp order and — worse — materializes the **entire** unified inbox (~thousands of rows) into
|
||||
memory on every emission. This measures that path **before** the fix and **with** Paging 3, on a large
|
||||
seeded cache, to decide whether an index (schema migration) is actually needed once paged.
|
||||
|
||||
## TL;DR / verdict
|
||||
|
||||
- **Paging fixes it; no index, no schema migration.** The current whole-unified-inbox query grows
|
||||
linearly with the number of INBOX rows (**~6.8 ms at 1k INBOX → ~24.6 ms at 4k INBOX**), because it
|
||||
reads and maps every INBOX row across all accounts. The paged first page (the production initial
|
||||
load of 120 rows) is **flat at ~5–7 ms regardless of total cache size** — it stops as soon as it has
|
||||
a screenful — a **~3.6× first-emit speedup at a 20k cache**, and, more importantly, it stays flat as
|
||||
the cache keeps growing (full-history backfill, #12/#13) while the current path keeps rising.
|
||||
- **`EXPLAIN QUERY PLAN` still shows a `SCAN … index_messages_timestampMillis`** for the paged query on
|
||||
the existing indices — i.e. no `folder`-leading index is used — yet the paged first page is already
|
||||
flat, because the `LIMIT` lets the planner stop early. A `(folder, timestampMillis)` /
|
||||
`(folder, inInbox, timestampMillis)` index would only materially help a **deep scroll** (large
|
||||
`OFFSET`), which is rare and already bounded by scroll depth (not total cache), and even the worst
|
||||
deep page measured (40 rows at offset ~3.9k) is **faster than the current whole-inbox load**. So the
|
||||
ticket's strong preference holds: **Paging 3 alone captures the win — no `(folder, …)` index, no
|
||||
v15→v16 migration, no #118 version coordination.**
|
||||
- **Per-account views are untouched** (already flat from #86); only the unified browse list is paged.
|
||||
|
||||
## Setup & method
|
||||
|
||||
- Device: Gradle-managed AVD `libremail_api29` (API 29, x86, google_apis), the same device #86 used, so
|
||||
the figures are comparable. Cross-checked on a physical Pixel (API 37) — same shape (see below).
|
||||
- Harness: a throwaway instrumented probe (`UnifiedInboxPagingProbe`, removed after measuring, like
|
||||
#86's) built the real `LibreMailDatabase` in-memory (real entities, real indices, real generated
|
||||
`MessageDao`), seeded it, and timed each variant with **5 warmup + 15 measured iterations**, reporting
|
||||
**median and min** wall-clock ms with a GC between phases. `androidx.benchmark` was deliberately not
|
||||
used (as in #86: on an emulator only the **relative** A/B result is meaningful, and it would force the
|
||||
module's instrumentation runner, changing what CI's E2E jobs run under).
|
||||
- Dataset: rows spread across **3 accounts × {INBOX, Sent, Archive, Spam, Work}** (INBOX ≈ 20%, so at
|
||||
20k the unified inbox is ~4k rows), timestamps interleaved across folders (a realistic worst case —
|
||||
INBOX rows are **not** clustered at the top of the timestamp index), each row carrying a ~1 KB body so
|
||||
rows are realistically sized (the summary projection never selects the body).
|
||||
- Variants: (a) **CURRENT** = `observeUnifiedFolderSummaries("INBOX")` first Flow emission (materializes
|
||||
the whole unified inbox); (b) **PAGED first page** = `pagingUnifiedFolderSummaries("INBOX")` loaded via
|
||||
a `Refresh(key = null, loadSize = 120)` — the production `initialLoadSize` (3 × pageSize); (c) **PAGED
|
||||
deep page** = a `Refresh(key ≈ inboxCount − 60, loadSize = 40)` window near the end of the inbox
|
||||
(models scrolling to the bottom). Each query's `EXPLAIN QUERY PLAN` was captured with existing indices
|
||||
only.
|
||||
|
||||
## Results (median / min ms, lower is better)
|
||||
|
||||
| total rows | INBOX rows | CURRENT whole-inbox first-emit | PAGED first page (120) | PAGED deep page (40 @ ~end) |
|
||||
|-----------:|-----------:|-------------------------------:|-----------------------:|----------------------------:|
|
||||
| 5 000 | 1 000 | **6.80** / 4.93 | 5.44 / 4.64 | 3.67 / 2.83 |
|
||||
| 20 000 | 4 000 | **24.56** / 18.89 | 6.82 / 5.67 | 12.86 / 10.52 |
|
||||
|
||||
- **CURRENT scales with the inbox size:** 6.8 → 24.6 ms as INBOX rows go 1k → 4k (≈ linear); this cost
|
||||
is paid on **every** re-emission (any write to `messages` — IDLE delivery, a read/star toggle, a
|
||||
backfill page, a sync of any folder), and it also builds a full `List<Message>` of every INBOX row.
|
||||
- **PAGED first page is flat:** ~5.4 → ~6.8 ms (within noise) — independent of the total cache, because
|
||||
the scan stops once it has 120 INBOX rows. This is the common case (opening / reading the top of the
|
||||
inbox), and it's what the acceptance criterion asks for.
|
||||
- **PAGED deep page** (40 rows at offset ~3.9k) costs 12.9 ms at 20k — more than the first page (the
|
||||
`OFFSET` walks ~3.9k INBOX matches) but still **below the current whole-inbox load**, returns only 40
|
||||
rows (vs. the current path's 4 000), and scales with **scroll depth**, not total cache.
|
||||
|
||||
Cross-check on a physical **Pixel (API 37)** at 20k: CURRENT 27.2 / 24.8, PAGED first page 8.6 / 7.2,
|
||||
PAGED deep page 15.3 / 12.4 ms — same shape (~3.2× first-page speedup).
|
||||
|
||||
## Query plans (`EXPLAIN QUERY PLAN`, existing indices only)
|
||||
|
||||
```
|
||||
CURRENT whole-inbox : SCAN TABLE messages USING INDEX index_messages_timestampMillis
|
||||
PAGED first page : SCAN TABLE messages USING INDEX index_messages_timestampMillis (LIMIT 120)
|
||||
PAGED deep page : SCAN TABLE messages USING INDEX index_messages_timestampMillis (LIMIT 40 OFFSET ~3.9k)
|
||||
```
|
||||
|
||||
- All three walk `index_messages_timestampMillis` newest-first (no `folder`-leading index), filtering
|
||||
`folder`/`inInbox` per row. The paged queries differ only by the `LIMIT`/`OFFSET` the planner applies,
|
||||
which is exactly what makes the first page cheap: it stops after a screenful.
|
||||
- A `(folder, inInbox, timestampMillis)` index would turn the scan into a seek and remove the deep-page
|
||||
`OFFSET` walk. It is **not added**: the first page — the case the ticket targets — is already flat on
|
||||
the existing indices, deep scroll is rare and bounded by depth, and avoiding the index avoids a
|
||||
schema migration (v15→v16) and the #118 version-coordination it would require. Revisit only if deep
|
||||
scrolling the unified inbox becomes a measured problem.
|
||||
|
||||
## What was implemented
|
||||
|
||||
- `MessageDao.pagingUnifiedFolderSummaries(folder)` — a Paging 3 `PagingSource<Int, MessageSummary>`
|
||||
over `WHERE folder = ? AND inInbox = 1 ORDER BY timestampMillis DESC` (synced rows only; unified
|
||||
**search** keeps using `observeUnifiedFolderSummaries`, which also surfaces transient `inInbox = 0`
|
||||
hits).
|
||||
- `MailRepository.pagedUnifiedFolderMessages(folder)` — wraps it in a `Pager`
|
||||
(`pageSize = 40`, `initialLoadSize = 120`, no placeholders) and maps summaries to domain.
|
||||
- `MailboxViewModel.pagedMessages` — `flatMapLatest` to the paged flow while browsing the unified inbox
|
||||
(no account selected, no active search), else empty paging data; `cachedIn(viewModelScope)`. The old
|
||||
`messages` list flow now stays **empty** while browsing the unified inbox, so the whole inbox is never
|
||||
pulled into memory; it still serves per-account views and unified **search**. Selection carries each
|
||||
row's `accountId` (captured at tap time) so "Move" still works without a full in-memory list.
|
||||
- `MailboxScreen` renders the unified browse list via `collectAsLazyPagingItems()`; per-account/search
|
||||
render the flat list as before.
|
||||
|
||||
## Reproduce
|
||||
|
||||
Seed a large multi-account `messages` cache, then compare `observeUnifiedFolderSummaries("INBOX")`
|
||||
(first emit) against `pagingUnifiedFolderSummaries("INBOX")` loaded with a `Refresh(null, 120)` on an
|
||||
emulator, and inspect `EXPLAIN QUERY PLAN`. The paged first page should be ~flat (~5–7 ms) regardless
|
||||
of total cache; the whole-inbox query should grow with the INBOX row count.
|
||||
@@ -114,9 +114,10 @@ UNIFIED folder-only : SCAN TABLE messages USING INDEX index_messages_timestampMi
|
||||
|
||||
## Deferred follow-ups
|
||||
|
||||
- **Unified inbox**: add Room `PagingSource`/Paging3 so the unified list's cost scales with the screen,
|
||||
not the total inbox count; optionally a `(folder, timestampMillis)` index to turn the scan into a
|
||||
seek. (The per-account views are already flat, so Paging3 there is lower priority.)
|
||||
- **Unified inbox**: ~~add Room `PagingSource`/Paging3 so the unified list's cost scales with the
|
||||
screen, not the total inbox count~~ — done in #124 (`docs/perf/issue-124-unified-inbox-paging.md`).
|
||||
Paging alone captured the win (first page ~flat at ~5–7 ms vs. the current ~25 ms at a 20k cache);
|
||||
the `(folder, timestampMillis)` index proved unnecessary, so no schema migration was added.
|
||||
- **IMAP latency on folder open** is out of scope for this cached-render fix.
|
||||
|
||||
## Reproduce
|
||||
|
||||
@@ -12,6 +12,7 @@ webkit = "1.12.1"
|
||||
biometric = "1.1.0"
|
||||
composeBom = "2026.06.00"
|
||||
room = "2.8.4"
|
||||
paging = "3.3.6"
|
||||
sqlcipher = "4.16.0"
|
||||
datastore = "1.2.1"
|
||||
work = "2.11.2"
|
||||
@@ -72,9 +73,18 @@ androidx-room-ktx = { group = "androidx.room", name = "room-ktx", version.ref =
|
||||
androidx-room-compiler = { group = "androidx.room", name = "room-compiler", version.ref = "room" }
|
||||
# Room MigrationTestHelper (instrumented migration tests).
|
||||
androidx-room-testing = { group = "androidx.room", name = "room-testing", version.ref = "room" }
|
||||
# Room PagingSource support — lets @Query methods return androidx.paging.PagingSource.
|
||||
androidx-room-paging = { group = "androidx.room", name = "room-paging", version.ref = "room" }
|
||||
# SQLCipher — opt-in at-rest encryption of the Room cache.
|
||||
sqlcipher-android = { group = "net.zetetic", name = "sqlcipher-android", version.ref = "sqlcipher" }
|
||||
|
||||
# Paging 3 — pages the unified "All inboxes" list so its query/recomposition cost scales with the
|
||||
# visible window, not the whole cache (issue #124). runtime = Pager/PagingData; compose =
|
||||
# collectAsLazyPagingItems; testing = asSnapshot for JVM unit tests.
|
||||
androidx-paging-runtime = { group = "androidx.paging", name = "paging-runtime", version.ref = "paging" }
|
||||
androidx-paging-compose = { group = "androidx.paging", name = "paging-compose", version.ref = "paging" }
|
||||
androidx-paging-testing = { group = "androidx.paging", name = "paging-testing", version.ref = "paging" }
|
||||
|
||||
# DataStore (settings) / WorkManager (sync) — wired in later increments
|
||||
androidx-datastore-preferences = { group = "androidx.datastore", name = "datastore-preferences", version.ref = "datastore" }
|
||||
androidx-work-runtime-ktx = { group = "androidx.work", name = "work-runtime-ktx", version.ref = "work" }
|
||||
|
||||
Reference in New Issue
Block a user