Perf(reader): stop manhwa scroll degrading after chapters and drawer use#72
Merged
Conversation
AsyncImage.onSuccess re-fires every time a recycled item re-enters composition (memory-cache hits included), so a fast up/down drag calls persistImageDimensions once per re-entry per image. Each call wrote a fresh Pair into the SnapshotStateMap (invalidating every composed reader of the map), re-enqueued a Room upsert of the same row, and cancelled/ relaunched the content-dim apply job — a per-frame storm during the exact gesture where scroll jank is most visible. Early-return when the stored dimensions already match, so revisited images no longer pay any of that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
SnapshotStateMap tracks reads per-map, not per-key, so every visible image (each reads the map in ReaderImageView and in the ScrollingReaderView item scope for slice count) was invalidated and recomposed whenever ANY image resolved its dimensions — despite the comment claiming per-key granularity. During a scroll through a fresh chapter that meant every decode recomposed the whole visible set. Replace the map with one MutableState per URL behind ConcurrentHashMap.computeIfAbsent (atomic, unlike getOrPut, so the State a composable subscribes to is always the one later writes land on). Read sites subscribe via remember(url) + State delegate, so a write now invalidates only that image's scope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
The dimension state was never cleared: ImageDimensionManager.reset() was only reachable from ReaderViewModel.resetState(), which has no callers, and the ViewModel is Activity-scoped — so the per-URL state grew by every image ever displayed, for the whole session. Replace reset() with pruneForChapter(currentImageUrls), called after a chapter load succeeds (web + epub paths). Only entries missing from the new chapter are dropped: a same-chapter reload keeps its dims, a failed navigation never prunes (the old chapter stays on screen), and revisited chapters re-seed declared dimensions from the Room cache at parse time. Pending DB flushes are left untouched so in-flight persists complete. Delete the dead resetState(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
getLikelyMediaState stats up to 3 candidate files (exists/length/mtime) and ReaderImageView called it twice per image item — on the main thread, in composition, re-executed every time a recycled item re-entered during a scroll gesture. Memoize the state string in ImageCache and invalidate it wherever a url's candidate files can change: the cache mutators, trimToSize/clearAll (whole-memo drop, trim deletes arbitrary files), and every terminal branch of WebContentLoader.downloadAndCacheImageInternal — the only production path that creates or replaces cached media. Staleness can never render a wrong image: HttpMediaCacheFetcher re-checks disk authoritatively; the string only feeds the Coil memory-cache key, the loading-UI choice, and repair eligibility. ReaderImageView now derives isInitiallyCached from localMediaState, so each composition entry performs one (memoized) probe instead of two disk probes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
Keying the prefetch LaunchedEffect on listState.firstVisibleItemIndex read the index during composition, so every item boundary crossed while scrolling recomposed the entire ContentArea scope and cancelled+ relaunched the effect coroutine. Observe the index via snapshotFlow in one persistent effect per chapter instead; composition no longer subscribes to per-frame scroll state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
Extend the existing helper to recurse into PageContent (no callers relied on the old shallow behavior — it had none) instead of adding a fourth private copy of the Image/ImageGroup traversal in ReaderViewModel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
…timing Adversarial review of the perf series surfaced four timing holes, each confirmed by several independent passes: - Media-state memo invalidation moved from the download awaiter's finally into the shared deferred's invokeOnCompletion. The awaiter can be cancelled (Coil disposes a request when the item scrolls away) before the file lands; the detached download then wrote the file with no invalidation, leaving a permanent stale 'missing' entry — placeholder flash on every re-entry of a fully cached image. - ImageCache mutators now invalidate AFTER mutating disk; invalidating first let a concurrent probe re-memoize the pre-mutation state. - trimToSize only clears the memo when files were actually deleted (and after the walk). Trim runs after chapter loads and every ~30s during scroll-driven prefetch and is usually a no-op — the unconditional clear was periodically defeating the memoization outright. - getLikelyMediaState serves hits with a plain lookup; the size guard (which can clear the map) runs only on the miss/insert path, matching the integrityVerdicts pattern. - pruneForChapter now runs after the new content is committed to uiState, so the still-composed outgoing chapter is never stripped mid-display and late decodes cannot resurrect just-pruned entries; it also re-enqueues surviving urls into the content rebuild so a chapter sharing an image with its predecessor still gets its element healed despite the duplicate-persist guard. - A failed Room dimension flush re-queues its batch (persistAll now reports success); the duplicate-persist guard would otherwise swallow every future chance to retry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gzsfvi9WhP4KJDvAhEqR9y
Refactor(tests): invalidate media state in ImageCacheTest after writing bytes Build(gradle): update gradle wrapper properties and scripts for consistency
Long-strip scroll micro-stuttered on fast up/down drags, got worse after a chapter or two, and stuttered almost every time after opening then closing the library drawer. - Coil memory cache raised 0.25 -> 0.5 of heap. Strips are region-decoded into ~9MB HARDWARE slices, so at 0.25 the LRU held under one chapter's worth and a fast up/down drag or scroll-back re-decoded evicted slices every time. - Reader drawer now reads a dedicated lean flow (sections + isEmpty) instead of the whole-library uiState combine, so opening it no longer group+sorts the entire library (getGroupedByTitle/BySource) on the drawer path. - Restore edge-swipe-to-open the drawer in continuous scroll; paged mode keeps the left edge for page turns. - Revert an unrelated restore-fallback change that dropped the percent self-heal for precise restores, and restore its tests. Supporting work kept from the same effort: library-state combine moved off the main thread with a lifecycle-scoped subscription, nested-scroll-connection and progress-save reworks to cut per-frame work during scroll, and an e2e suite covering scroll, drawer and state restoration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes long-strip scroll micro-stutter that got worse after a chapter or two and stuttered almost every time after opening then closing the library drawer. Worst on fast up/down drags.
Changes
uiStatecombine, so opening it no longer group+sorts the entire library on the drawer path.Also on this branch (earlier reader-perf commits)
Per-URL image-dimension state with per-chapter pruning, prefetch driven by
snapshotFlow, library-state combine moved off the main thread with a lifecycle-scoped subscription, nested-scroll-connection and progress-save reworks to cut per-frame scroll work, and an e2e suite covering scroll, drawer and state restoration.Verification
testStandardDebugUnitTest+detektgreen. Drawer and fast up/down confirmed smooth on-device.