From 14615f984289c2ac5b4126c9b13ded51a5569233 Mon Sep 17 00:00:00 2001 From: Hadrian Burkhardt Date: Sat, 17 Oct 2026 16:11:28 +0200 Subject: [PATCH] Take the two main-thread disk reads off the startup path, and report what that bought Both costs StrictMode found on a cold start are now off the main thread. scratchRoot is a Lazy, so Context.getCacheDir - which checks and creates the private cache directory, measured at 492 ms - runs when a preview first copies something instead of while the view model is being created. The free-space probes moved into fillRootFreeSpace and fillFreeSpace, which run on Dispatchers.IO after the rows are already on screen and update the state when the filesystem answers; nothing waits for them, and fillFreeSpace checks the path is still the one on screen so a fast double-tap cannot leave a folder showing another one's number. Measured after: StrictMode no longer reports either violation, and the only ones left are MIUI's own font hook - FontSettings.loadFontSetting, TypefaceUtils.loadFontSettings, InterceptorProxy.create via MainActivity.onCreate, in 50-78 ms pairs. The debug cold start went from a 1814 ms median to 1803 ms and the benchmark coldStart from 519.5/531.6 ms to 485.6/568.9/600.0 ms. That is no improvement, and PERFORMANCE.md says so in those words: both violations were first-call costs (cache directory creation once per install, a cold StatFs page cache), the timed runs are warm, and the device's run-to-run spread of 115 ms is wider than anything the change could have bought. What it does change is the launch right after an install, which is the benchmark's first iteration: 648 ms max fresh against 550 ms warm. --- .../ui/browser/BrowserViewModel.kt | 62 +++++++++++++++++-- .../ui/browser/BrowserViewModelTest.kt | 4 +- docs/CODE_MAP.md | 4 +- docs/PERFORMANCE.md | 33 ++++++++-- 4 files changed, 91 insertions(+), 12 deletions(-) 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