fishfacegit/file-manager
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Take the sort of a large folder off the main thread

Browse Source 
The 20000-file measurement had a shape worth following up: median 184 frames, but one iteration of five at 598. Reading the listing path found the reason it could be that shape. AllFilesSource.list already hops to Dispatchers.IO, but BrowserViewModel.recompute filtered and sorted whatever it returned on the caller's thread - and the default order is natural order, which parses the digits in every name on every comparison, so 20000 rows is a few hundred thousand string comparisons plus a fresh 20000-element list inside a tap.

recompute now sorts off the main thread above 500 rows and keeps the synchronous path below it, so the common case is untouched and the tests that read state immediately after an action still hold. The dispatcher is injected for exactly that reason, and the large path has its own test: 600 files, listed, in natural order. The state update is guarded on path, sort and hidden-files so a second tap cannot be overwritten by the answer to the first.

The fix is not re-measured: the 20000-file fixture takes half an hour to build through MIUI's storage layer, so PERFORMANCE.md keeps the 598 outlier as the motivation and says plainly that whoever has the fixture next should confirm the maximum came down.
This commit is contained in:
Hadrian Burkhardt
2026-10-17 20:34:39 +02:00
parent 65569e9af2
commit e5c1ab10a7
4 changed files with 97 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt
+57 -3
View File
@@ -51,6 +51,7 @@ import de.softwareapp_hb.filemanager.domain.TrashedEntry
import de.softwareapp_hb.filemanager.domain.UsageAnalysis
import de.softwareapp_hb.filemanager.domain.UsageNode
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.currentCoroutineContext
@@ -101,9 +102,20 @@ class BrowserViewModel(
* disables those previews.
*
* A `File` rather than a `Context`: the ViewModel keeps its distance from the platform,
* and the application hands it a directory it already owns.
* and the application hands it a directory it already owns. [Lazy] because resolving it
* means `Context.getCacheDir`, which checks and creates the directory — measured at 492 ms
* on the first start (docs/PERFORMANCE.md), and never needed until a preview copies
* something.
*/
private val scratchRoot: Lazy<File>? = null,
/**
* Where a folder too large to sort on the main thread is sorted instead.
*
* Injected so a test can make the sort synchronous: the behaviour under test is "a big folder is
* sorted and shown", not which thread did it, and `UnconfinedTestDispatcher` keeps the assertions
* where they were.
*/
private val io: CoroutineDispatcher = Dispatchers.IO,
) : ViewModel() {
private val _state = MutableStateFlow(restoredViewState())
@@ -313,12 +325,44 @@ class BrowserViewModel(
* that streams its own matches into [BrowserUiState.entries]; this is only ever the
* "what is in this folder" view.
*/
/**
* Derives what is shown from [listing]: hidden files filtered out, then sorted.
*
* A big folder is sorted on [io] instead of on the caller's thread. `File.listFiles` and the entry
* data are already read off the main thread, but the *sort* was not: the default order is natural
* order, which parses the digits in every name on every comparison, so twenty thousand rows means
* a few hundred thousand string comparisons in the middle of a tap. Measured on the phone at that
* size, one iteration out of five took 598 frames against a median of 184 — the kind of hitch this
* moves off the frame. Small folders stay on the caller's thread, which keeps the code (and the
* tests) honest for the case that happens all the time.
*/
private fun recompute() {
val current = _state.value
val shown = listing.filter { current.showHidden || !it.isHidden }
_state.update { it.copy(entries = current.sort.sort(shown)) }
if (listing.size < SORT_OFF_MAIN_THRESHOLD) {
_state.update { it.copy(entries = current.sort.sort(visible(current))) }
return
}
viewModelScope.launch {
val sorted = withContext(io) { current.sort.sort(visible(current)) }
// Only if the screen still shows that folder, in that order: a second tap must not be
// overwritten by the answer to the first.
_state.update { state ->
if (state.path == current.path &&
state.sort == current.sort &&
state.showHidden == current.showHidden
) {
state.copy(entries = sorted)
} else {
state
}
}
}
}
private fun visible(state: BrowserUiState): List<FileEntry> =
listing.filter { state.showHidden || !it.isHidden }
/**
* The trail from the browsable root down to [path].
*
@@ -2108,6 +2152,16 @@ class BrowserViewModel(
* them — they would fill the candidate list without ever finding space worth having. 1 KB
* is where "this is a real file" starts.
*/
/**
* Above this many rows, a folder is filtered and sorted off the main thread.
*
* The number is where a sort stops being free: a few hundred rows sort in well under a frame,
* and a folder of thousands does not — twenty thousand rows of natural-order comparison is a
* few hundred thousand string comparisons. Small folders therefore keep the simple synchronous
* path, and only the folders that can hitch take the detour.
*/
const val SORT_OFF_MAIN_THRESHOLD = 500
const val DUPLICATE_MIN_BYTES = 1024L
/**
app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt
+22
View File
@@ -33,6 +33,7 @@ import de.softwareapp_hb.filemanager.domain.SearchIndexer
import de.softwareapp_hb.filemanager.domain.testutil.InMemorySearchIndex
import de.softwareapp_hb.filemanager.domain.testutil.InMemoryStorageSource
import de.softwareapp_hb.filemanager.ui.browser.testutil.FakeStorageLocation
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.delay
@@ -135,6 +136,9 @@ class BrowserViewModelTest {
searchIndex: SearchIndex = SearchIndex.None,
bookmarks: BookmarkStore = InMemoryBookmarkStore(),
clock: () -> Long = System::currentTimeMillis,
// Unconfined in tests: the view model sorts a large folder on its own dispatcher, and the
// assertions in this file are written to read the state immediately after an action.
io: CoroutineDispatcher = UnconfinedTestDispatcher(),
) = BrowserViewModel(
storage = location,
notifier = recordingNotifier,
@@ -145,6 +149,7 @@ class BrowserViewModelTest {
searchIndex = searchIndex,
bookmarkStore = bookmarks,
clock = clock,
io = io,
// The view model takes it lazily now — `app.cacheDir` is a disk check that used to run while
// the screen was being built — so the test wraps the folder it already made.
scratchRoot = scratchRoot?.let { folder -> lazyOf(folder) },
@@ -212,6 +217,23 @@ class BrowserViewModelTest {
assertEquals(listOf("b.txt", "a.txt"), model.state.value.entries.map { it.name })
}
@Test
fun `a folder of six hundred files is still listed in natural order`() {
val big = root.child("big")
storage.addDirectory(big)
for (index in 0 until 600) storage.addFile(big.child("file-$index.txt"), "x")
val model = viewModel()
model.openDirectory(big)
val names = model.state.value.entries.map { it.name }
assertEquals(600, names.size)
// Natural order rather than lexicographic, which is the whole point of the comparator — and
// the case that has to survive being moved onto another dispatcher.
assertEquals(listOf("file-0.txt", "file-1.txt", "file-2.txt"), names.take(3))
assertEquals("file-599.txt", names.last())
}
@Test
fun `navigating up goes to the parent then to the roots`() {
val model = viewModel()