diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt index fe1afb2..47de09a 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt @@ -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.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.flow.MutableStateFlow @@ -59,6 +60,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.io.File /** @@ -101,7 +103,7 @@ class BrowserViewModel( * A `File` rather than a `Context`: the ViewModel keeps its distance from the platform, * and the application hands it a directory it already owns. */ - private val scratchRoot: File? = null, + private val scratchRoot: Lazy? = null, ) : ViewModel() { private val _state = MutableStateFlow(restoredViewState()) @@ -142,6 +144,9 @@ class BrowserViewModel( ) } + // The rows are on screen; their free-space numbers follow when the filesystem answers. + viewModelScope.launch { fillRootFreeSpace() } + if (source == null) { listing = emptyList() recompute() @@ -277,6 +282,8 @@ class BrowserViewModel( // Navigating abandons any search: its results belonged to the folder being left. searchJob?.cancel() libraryJob?.cancel() + // 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()) @@ -290,7 +297,7 @@ class BrowserViewModel( query = "", searchRoot = null, searching = false, - freeBytes = storage.space(path)?.freeBytes, + freeBytes = null, loading = false, ) } @@ -344,11 +351,53 @@ class BrowserViewModel( path = volume.root, labelRes = if (volume.label == null) R.string.browser_internal_storage else null, label = volume.label, - freeBytes = storage.space(volume.root)?.freeBytes, + // No `freeBytes` here on purpose. Reading it means `StatFs`, which StrictMode + // measured at 397 ms on the first call (docs/PERFORMANCE.md), and this function runs + // while the root screen is being built. [fillRootFreeSpace] adds it a moment later. ) } } + /** + * The free-space line on each root, once the filesystem has answered. + * + * The rows are drawn without it, so the screen appears with the volumes and the numbers arrive a + * frame or two later — a number that is late reads far better than a start that is half a second + * late. The probe is off the main thread, which is the whole point: `StatFs` on the main thread is + * what the cold-start measurement found. + */ + private suspend fun fillRootFreeSpace() { + // A granted tree is a single root whose free space the platform does not report anyway. + if (storage.treeLabel() != null) return + val roots = _state.value.roots + if (roots.isEmpty()) return + + val free = withContext(Dispatchers.IO) { + roots.associate { it.path to runCatching { storage.space(it.path)?.freeBytes }.getOrNull() } + } + _state.update { state -> + state.copy( + roots = state.roots.map { root -> + free[root.path]?.let { root.copy(freeBytes = it) } ?: root + }, + ) + } + } + + /** + * The free space of the folder on screen, read off the main thread. + * + * Same reasoning as [fillRootFreeSpace]: the listing is shown first, and the number follows. + */ + private suspend fun fillFreeSpace(path: StoragePath) { + val free = withContext(Dispatchers.IO) { + runCatching { storage.space(path)?.freeBytes }.getOrNull() + } + // Only if the screen is still this folder: a fast tap through two folders must not leave the + // second one showing the first one's free space. + _state.update { state -> if (state.path == path) state.copy(freeBytes = free) else state } + } + // ----------------------------------------------------------------- libraries /** @@ -1457,7 +1506,7 @@ class BrowserViewModel( /** A file to copy one entry into, or `null` when this build has nowhere to put one. */ private fun reserveScratch(name: String): File? { - val root = scratchRoot ?: return null + val root = scratchRoot?.value ?: return null // One folder per preview, holding the entry under its own name — a viewer shows the // file's name in its title bar, and a copy called `0-shot.png` would read as a file @@ -2084,7 +2133,10 @@ class BrowserViewModel( bookmarkStore = app.bookmarkStore, apkInspector = app.apkInspector, searchIndex = app.searchIndex, - scratchRoot = app.cacheDir.resolve(SCRATCH_DIRECTORY), + // Lazy on purpose: `app.cacheDir` checks and creates the private cache + // directory, which StrictMode measured at 492 ms on the first start — see + // docs/PERFORMANCE.md. Nothing needs it until a preview copies something. + scratchRoot = lazy { app.cacheDir.resolve(SCRATCH_DIRECTORY) }, ) } } diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt index a58f2de..533f088 100644 --- a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt @@ -145,7 +145,9 @@ class BrowserViewModelTest { searchIndex = searchIndex, bookmarkStore = bookmarks, clock = clock, - scratchRoot = scratchRoot, + // 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) }, ) /** A real ZIP in memory, so an entry can be previewed out of something that has an index. */ diff --git a/docs/CODE_MAP.md b/docs/CODE_MAP.md index 2d0731a..4c1268a 100644 --- a/docs/CODE_MAP.md +++ b/docs/CODE_MAP.md @@ -2,7 +2,7 @@ Generated from the source by `tools/make-code-map.py` - do not edit by hand. Regenerate it after a change (`python3 tools/make-code-map.py`); `--check` exits non-zero when this file is stale. -77 source files, 12741 lines - 33 test files, 386 tests. The behaviour index at the end lists every test name, and in this project a test name is a sentence about what the app does, so it doubles as the specification. +77 source files, 12793 lines - 33 test files, 386 tests. The behaviour index at the end lists every test name, and in this project a test name is a sentence about what the app does, so it doubles as the specification. Start at `AGENTS.md`. What is done, and why: `docs/ROADMAP.md` and `CHANGELOG.md`. What was verified by hand rather than by a test: `docs/EDGE_CASE_TESTING.md`. @@ -101,7 +101,7 @@ The browser screen: view model, state, list and grid, dialogs, selection bar | `BrowserScreen.kt` | 591 | BrowserScreen() | - | | `BrowserTopBar.kt` | 238 | The browser's app bar: title, up, the favourite star, and the search / sort / overflow actions. BrowserTopBar(), SEARCH_FIELD_WIDTH | `BrowserTopBarTest` (4) | | `BrowserUiState.kt` | 311 | One immutable description of everything the browser shows. ViewMode, SearchScope, Clipboard, UiMessage, ConflictPrompt, RootItem, Bookmark, DetailFocus, Undo, UndoMove, Overlay, BrowserUiState | - | -| `BrowserViewModel.kt` | 2100 | Drives the browser screen. BrowserViewModel, ArchivePreview | `BrowserViewModelTest` (89) | +| `BrowserViewModel.kt` | 2152 | Drives the browser screen. BrowserViewModel, ArchivePreview | `BrowserViewModelTest` (89) | | `DetailsSheet.kt` | 109 | Name, location, size, type and modified date for one entry. DetailsSheet(), DetailLines(), DetailLine() | - | | `EntryIntents.kt` | 58 | Wraps an entry's path in a content:// URI another app may read. contentUriFor(), viewIntent(), shareIntent() | - | | `IncomingBar.kt` | 60 | The bar shown while another app is waiting on this one. IncomingBar() | - | diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md index 8601421..b3f0423 100644 --- a/docs/PERFORMANCE.md +++ b/docs/PERFORMANCE.md @@ -46,10 +46,35 @@ The same debug build now logs StrictMode violations, and the first cold start pr | 477 ms | `FileManagerTheme` → `typography = FileManagerTypography` | no — the platform loading its own typefaces on first composition | | 54 ms × 2 | `miui.util.font.FontSettings` via `MainActivity.onCreate` | no — the OEM's font hook doing `File.exists` while the activity is created | -That is **~1.5 s of main-thread disk I/O inside a 1.8 s start**, of which **~0.9 s is ours**. Both of -ours are mechanical to move and neither is fixed yet: the cache directory is only needed when a preview -copies something, and the free-space line on the roots screen can arrive a frame later. They are the -first items of the speed work. +That is **~1.5 s of main-thread disk I/O inside a 1.8 s start**, of which **~0.9 s was ours**. Both of +ours were mechanical to move — the cache directory is only needed when a preview copies something, and the +free-space line can arrive a frame later — and both are fixed now; what they bought, measured, is below. + +### After the fix: what moved, and what did not + +Both are now fixed — `scratchRoot` is a `Lazy` resolved on first use, and the free-space probes run +in `Dispatchers.IO` and update the state when they return (`fillRootFreeSpace`, `fillFreeSpace`). Measured +afterwards: + +| what | before | after | +|---|---|---| +| StrictMode at start, ours | 492 ms `cacheDir`, 397 ms `StatFs` | **gone** | +| StrictMode at start, not ours | 477 ms typefaces, 54 ms × 2 MIUI font hook | 50–78 ms in pairs, all from `miui.util.font.FontSettings.loadFontSetting` / `TypefaceUtils.loadFontSettings` / `contentcatcher.InterceptorProxy.create` via `MainActivity.onCreate` | +| debug cold start, 5 runs | median 1 814 ms | median 1 803 ms (1 697 · 1 752 · 1 803 · 1 823 · 2 507) | +| `coldStart` on the benchmark build | 519.5 / 531.6 ms | 485.6 / 568.9 / 600.0 ms | + +**The numbers did not improve, and that is worth stating rather than dressing up.** The two violations were +*first-call* costs: creating the cache directory happens once per install, and `StatFs` is slow when its +page cache is cold. Neither is paid on a warm device, which is what all five timed runs are — and the +run-to-run spread on this phone (±115 ms between the three benchmark runs above) is larger than anything +the change could have bought. What the fix does buy is that the main thread no longer does this work at +all, which is exactly the case the benchmark's *first* iteration shows: its max is 648 ms after a fresh +install, against 550 ms on a warm one. + +The honest conclusion is that the remaining 1.8 s of a debug start is not made of these two calls. It is +R8-less code, Compose's first composition, the typefaces, and MIUI's font hook — and the release-like +build already pays 0.5 s, not 1.8 s. Making a *number* out of any further work needs either a quieter +device or many more runs; making the *app* faster needs a baseline profile (next section). ## Measured on demand: the `:benchmark` module