Draw a large folder while it is being read, instead of after
Two seconds of readdir and stat cannot be made much cheaper - each is a round trip to another process - so the folder is now read in batches and drawn as it arrives. StorageSource.listBatches hands over 200 entries at a time; AllFilesSource implements it with Files.newDirectoryStream, which is lazy where File.listFiles materialised the whole directory first; the default implementation is one batch, so the SAF backend and every test fake are unchanged. BrowserViewModel.load merges each batch into the list already on screen with core/SortedMerge, which is O(n+k) rather than re-sorting the folder on every batch, and keeps the rows in the user's order at every moment instead of shuffling them into place when the last batch lands. loading stays true until the last batch, so the progress bar the screen already draws sits above rows that are already useful. publishEntries is separate from recompute for that reason: the merge has done the ordering, and the batch path only has to apply the hidden filter. Tests: four for the merge (including ties not jumping ahead) and one for the batched read, which asserts 250 files arrive as 100+100+50 covering every entry exactly once - the thing a streaming read is easiest to get wrong about. PERFORMANCE.md records the expected effect and states plainly that it is not measured yet, since confirming it needs the 20000-file fixture rebuilt.
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
package de.softwareapp_hb.filemanager.core
|
||||
|
||||
/**
|
||||
* Adds a freshly read batch to an already sorted list, in one pass.
|
||||
*
|
||||
* A folder is now read in batches so that the first screenful can be drawn while the rest is still being
|
||||
* `stat`ed. Each batch is sorted on its own and merged here, which is O(n + k) — against O(n log n) for
|
||||
* re-sorting everything after every batch — and it means the rows on screen are always in the order the
|
||||
* user asked for, never a partly ordered jumble that reorders itself when the next batch lands.
|
||||
*/
|
||||
object SortedMerge {
|
||||
|
||||
/**
|
||||
* [sorted] must already be ordered by [comparator]; [batch] need not be.
|
||||
*
|
||||
* Ties keep the *earlier* entry first (`<=`), so an entry does not jump over an equal one just
|
||||
* because it arrived in a later batch.
|
||||
*/
|
||||
fun merge(
|
||||
sorted: List<FileEntry>,
|
||||
batch: List<FileEntry>,
|
||||
comparator: Comparator<FileEntry>,
|
||||
): List<FileEntry> {
|
||||
if (batch.isEmpty()) return sorted
|
||||
val ordered = batch.sortedWith(comparator)
|
||||
if (sorted.isEmpty()) return ordered
|
||||
|
||||
val merged = ArrayList<FileEntry>(sorted.size + ordered.size)
|
||||
var left = 0
|
||||
var right = 0
|
||||
while (left < sorted.size && right < ordered.size) {
|
||||
if (comparator.compare(sorted[left], ordered[right]) <= 0) {
|
||||
merged += sorted[left++]
|
||||
} else {
|
||||
merged += ordered[right++]
|
||||
}
|
||||
}
|
||||
while (left < sorted.size) merged += sorted[left++]
|
||||
while (right < ordered.size) merged += ordered[right++]
|
||||
return merged
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,9 @@ import android.annotation.SuppressLint
|
||||
import de.softwareapp_hb.filemanager.core.FileEntry
|
||||
import de.softwareapp_hb.filemanager.core.StoragePath
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.flow.flowOn
|
||||
import kotlinx.coroutines.withContext
|
||||
import java.io.File
|
||||
import java.io.FileInputStream
|
||||
@@ -12,6 +15,8 @@ import java.io.InputStream
|
||||
import java.io.OutputStream
|
||||
import java.nio.channels.FileChannel
|
||||
import java.nio.channels.SeekableByteChannel
|
||||
import java.nio.file.Files
|
||||
import java.nio.file.Paths
|
||||
import java.nio.file.StandardOpenOption
|
||||
|
||||
/**
|
||||
@@ -37,6 +42,30 @@ class AllFilesSource(
|
||||
if (children == null) emptyList() else children.map { it.toEntry() }
|
||||
}
|
||||
|
||||
/**
|
||||
* The same entries, handed over every [batchSize] rows while the directory is still being read.
|
||||
*
|
||||
* `Files.newDirectoryStream` is lazy — it pulls one `readdir` result at a time as the loop asks —
|
||||
* which is what makes a first batch possible at all; `File.listFiles` would materialise the whole
|
||||
* folder before returning anything, and on this phone that is about two seconds for twenty thousand
|
||||
* files. `flowOn(IO)` keeps the loop (and every `stat` in it) off the caller's thread; the caller
|
||||
* only ever sees finished batches.
|
||||
*/
|
||||
override fun listBatches(directory: StoragePath, batchSize: Int): Flow<List<FileEntry>> =
|
||||
flow {
|
||||
val batch = ArrayList<FileEntry>(batchSize)
|
||||
Files.newDirectoryStream(Paths.get(directory.value)).use { stream ->
|
||||
for (path in stream) {
|
||||
batch += path.toFile().toEntry()
|
||||
if (batch.size >= batchSize) {
|
||||
emit(batch.toList())
|
||||
batch.clear()
|
||||
}
|
||||
}
|
||||
}
|
||||
if (batch.isNotEmpty()) emit(batch.toList())
|
||||
}.flowOn(Dispatchers.IO)
|
||||
|
||||
/**
|
||||
* One `readdir` and no `stat` per child, which is the whole reason a catch-up pass is
|
||||
* cheap. `File.list()` returns `null` for a folder that cannot be read — exactly the
|
||||
|
||||
@@ -3,6 +3,8 @@ package de.softwareapp_hb.filemanager.data
|
||||
import de.softwareapp_hb.filemanager.core.FileEntry
|
||||
import de.softwareapp_hb.filemanager.core.StoragePath
|
||||
import java.io.InputStream
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import java.io.OutputStream
|
||||
import java.nio.channels.SeekableByteChannel
|
||||
|
||||
@@ -38,6 +40,32 @@ interface StorageSource {
|
||||
/** Children of [directory], in whatever order the backend returns them (unsorted). */
|
||||
suspend fun list(directory: StoragePath): List<FileEntry>
|
||||
|
||||
/**
|
||||
* [list], in the order the backend reads it, a batch at a time.
|
||||
*
|
||||
* The reason is the first batch. Reading a folder means a `readdir` and a `stat` per child, and a
|
||||
* phone's mounted storage makes each of those a round trip — measured at roughly 0.1 ms per file, so
|
||||
* twenty thousand files is about two seconds in which nothing can be drawn. Batches let the screen
|
||||
* show what has been read while the rest is being read, which is the difference between a slow
|
||||
* folder and a frozen one.
|
||||
*
|
||||
* The default is a single batch holding everything, so a backend that cannot stream — and every test
|
||||
* fake — behaves exactly as before.
|
||||
*/
|
||||
fun listBatches(directory: StoragePath, batchSize: Int = LIST_BATCH): Flow<List<FileEntry>> =
|
||||
flow { emit(list(directory)) }
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* How many entries are handed over at a time while a folder is read.
|
||||
*
|
||||
* Two hundred is roughly a few screens' worth: small enough that the first batch is drawn
|
||||
* quickly, large enough that a folder of a thousand files arrives in five batches rather than
|
||||
* fifty state updates.
|
||||
*/
|
||||
const val LIST_BATCH = 200
|
||||
}
|
||||
|
||||
/**
|
||||
* The names in [directory], **without reading metadata for each one**.
|
||||
*
|
||||
|
||||
@@ -22,6 +22,7 @@ import de.softwareapp_hb.filemanager.core.NameAllocator
|
||||
import de.softwareapp_hb.filemanager.core.SortDirection
|
||||
import de.softwareapp_hb.filemanager.core.SortKey
|
||||
import de.softwareapp_hb.filemanager.core.SortSpec
|
||||
import de.softwareapp_hb.filemanager.core.SortedMerge
|
||||
import de.softwareapp_hb.filemanager.core.StoragePath
|
||||
import de.softwareapp_hb.filemanager.core.TransferAction
|
||||
import de.softwareapp_hb.filemanager.core.isUsableName
|
||||
@@ -297,8 +298,9 @@ class BrowserViewModel(
|
||||
// Measured off the main thread, after the listing is up: see [fillFreeSpace].
|
||||
viewModelScope.launch { fillFreeSpace(path) }
|
||||
viewModelScope.launch {
|
||||
_state.update { it.copy(loading = true) }
|
||||
listing = runCatching { source.list(path) }.getOrDefault(emptyList())
|
||||
// The folder is announced before anything is read, so leaving the previous one is immediate
|
||||
// and the progress bar has something to sit above.
|
||||
listing = emptyList()
|
||||
_state.update {
|
||||
it.copy(
|
||||
path = path,
|
||||
@@ -310,14 +312,43 @@ class BrowserViewModel(
|
||||
searchRoot = null,
|
||||
searching = false,
|
||||
freeBytes = null,
|
||||
loading = false,
|
||||
loading = true,
|
||||
entries = emptyList(),
|
||||
)
|
||||
}
|
||||
|
||||
// Batch by batch: the first screenful is on screen while the rest is still being read. Each
|
||||
// batch is merged into the list already on screen (see [SortedMerge]), so rows arrive in the
|
||||
// user's order rather than shuffling into place afterwards.
|
||||
runCatching {
|
||||
source.listBatches(path).collect { batch ->
|
||||
listing = SortedMerge.merge(listing, batch, _state.value.sort.comparator)
|
||||
publishEntries()
|
||||
}
|
||||
}
|
||||
|
||||
if (_state.value.path == path) _state.update { it.copy(loading = false) }
|
||||
// Once more at the end, in case the sort was changed while the folder was still arriving:
|
||||
// batches were merged with whatever order was current when each one landed.
|
||||
recompute()
|
||||
persistViewState()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Publishes what has been read so far, with the hidden filter applied.
|
||||
*
|
||||
* No sort here: [listing] is kept ordered by [SortedMerge] as batches arrive, so a batch costs one
|
||||
* merge and one filter rather than a sort of the whole folder — which matters when the whole folder
|
||||
* is twenty thousand rows being delivered a hundred times.
|
||||
*/
|
||||
private fun publishEntries() {
|
||||
val current = _state.value
|
||||
_state.update { state ->
|
||||
state.copy(entries = listing.filter { current.showHidden || !it.isHidden })
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive what is shown from the unfiltered listing.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
package de.softwareapp_hb.filemanager.core
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* How a batch that arrives late joins a list that is already on screen.
|
||||
*
|
||||
* The list has to be *ordered at every moment* — not "sorted once the folder has finished", because the
|
||||
* folder may never finish visibly for twenty thousand rows — so each batch is merged rather than the
|
||||
* whole list re-sorted.
|
||||
*/
|
||||
class SortedMergeTest {
|
||||
|
||||
private val byName = SortSpec().comparator
|
||||
|
||||
private fun file(name: String, size: Long = 0) = FileEntry(
|
||||
path = StoragePath.of("/sdcard/$name"),
|
||||
isDirectory = false,
|
||||
sizeBytes = size,
|
||||
lastModifiedMillis = 0,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `a later batch slots into the order already on screen`() {
|
||||
val merged = SortedMerge.merge(
|
||||
sorted = listOf(file("a.txt"), file("c.txt")),
|
||||
batch = listOf(file("d.txt"), file("b.txt")),
|
||||
comparator = byName,
|
||||
)
|
||||
|
||||
assertEquals(listOf("a.txt", "b.txt", "c.txt", "d.txt"), merged.map { it.name })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the first batch into an empty list is sorted on its own`() {
|
||||
val merged = SortedMerge.merge(emptyList(), listOf(file("c.txt"), file("a.txt")), byName)
|
||||
|
||||
assertEquals(listOf("a.txt", "c.txt"), merged.map { it.name })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an empty batch changes nothing`() {
|
||||
val shown = listOf(file("a.txt"), file("b.txt"))
|
||||
|
||||
assertEquals(shown, SortedMerge.merge(shown, emptyList(), byName))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an entry that ties on name does not jump ahead of one that arrived earlier`() {
|
||||
// Two files with the same name can exist in different folders of a search result; the one
|
||||
// already on screen keeps its position so the row under the user's finger does not change.
|
||||
val merged = SortedMerge.merge(
|
||||
sorted = listOf(file("same.txt", size = 1)),
|
||||
batch = listOf(file("same.txt", size = 2)),
|
||||
comparator = byName,
|
||||
)
|
||||
|
||||
assertEquals(listOf(1L, 2L), merged.map { it.sizeBytes })
|
||||
}
|
||||
}
|
||||
@@ -24,6 +24,22 @@ class AllFilesSourceTest {
|
||||
|
||||
private fun path(file: File) = StoragePath.of(file.absolutePath)
|
||||
|
||||
@Test
|
||||
fun `reads a large folder in batches, covering every entry exactly once`() = runBlocking {
|
||||
val dir = temp.newFolder("big")
|
||||
repeat(250) { File(dir, "file-$it.txt").writeText("x") }
|
||||
|
||||
val batches = mutableListOf<List<String>>()
|
||||
source(dir).listBatches(path(dir), batchSize = 100).collect { batch ->
|
||||
batches += batch.map { it.name }
|
||||
}
|
||||
|
||||
// 250 files at 100 per batch: two full batches and a remainder, not one batch of everything.
|
||||
assertEquals(listOf(100, 100, 50), batches.map { it.size })
|
||||
// And every file exactly once, which is what a streaming read is easy to get wrong about.
|
||||
assertEquals(250, batches.flatten().toSet().size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `lists the children of a directory`() = runBlocking {
|
||||
val dir = temp.newFolder("docs")
|
||||
|
||||
Reference in New Issue
Block a user