You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
perf(home): fix slow home load on cold start and on return
Home took ~5s to populate when returning from Watchlist/TV/Search, and intermittently
5–10s on cold start. Four causes:
1. The HTTP disk cache never ran.OkHttpProvider.client is built lazily and memoised,
and OkHttpProvider.init() was called from Application.onCreate()aftersuper.onCreate()
— where Hilt's field injection already constructs Retrofit and therefore the client. So it was
always built with no app context and ran cacheless for the whole process (cache/http_cache
never existed on device). Moved to attachBaseContext, and init now falls back to the passed
context because getApplicationContext() returns null that early. Added a warning log when the
client builds without a context — it caught the second half of this.
2. navigateHome() destroyed HomeViewModel.navigateTopLevel leaves Home on the back stack
(inclusive = false), but the return path re-navigated with inclusive = true, killing the NavBackStackEntry that hiltViewModel() scopes to — so the screen rebuilt from scratch every
time. Now pops back to the existing entry, with the old navigate as fallback. popBackStack also
clears anything above Home, which handles the stale-Details case the old comment cited.
3. A startup race discarded the cached rows (the intermittent cold start). Two coroutines
start together in HomeViewModel.init: one restores the cached category rows from disk, the
other restores Continue Watching. The categories publish was guarded by _uiState.value.categories.isEmpty() — so whenever Continue Watching won the race and inserted
its row first, the guard failed and every cached base row was silently dropped. The screen
then showed Continue Watching alone until the network load finished, which is itself held behind
a 4–5.7s startup settle delay. Whichever coroutine finished first decided the outcome, which is
why it only happened sometimes. The guard now tests for real base rows
(hasRealBaseCategories(), mirroring what loadHomeData already uses), keeps a live Continue
Watching row over the cached copy, and preserves a hero it had already set.
4. Nothing would refresh afterwards. The rebuild had been doubling as the refresh mechanism.
Added a 6h staleness check on resume (refreshHomeDataIfStale); Continue Watching keeps its own
45s refresh.
Also: a network interceptor marking unsuccessful responses no-store, so a transient 5xx or a
429 can't be cached and replayed.
Measured on device
Return to Home — before: cache/http_cache absent, ~550 requests per return, ~5s.
After: 464 of 510 responses served from disk, 3 network calls; return is instant.
Cold start — captured a failing run at 5,881ms to first real content (cached rows
dropped, content only arriving with the network load). After the fix, 6/6 cold starts landed
at 318–458ms, publishing all 14 cached rows.
Thanks for this contribution. The Home caching/navigation changes look like a useful performance improvement and the PR is close to ready. I found two small edge cases that should be fixed before merging:
lastHomeDataLoadAtMs is updated before loadHomeData() succeeds. If the request fails temporarily, Home is still considered fresh and automatic resume refreshes can be skipped for six hours. Please update the timestamp only after a successful load, or restore/reset it in the failure path.
In the cached-category race fix, heroItem and heroLogoUrl are preserved independently. If Continue Watching sets a hero but has no cached logo, the base-cache path can keep that hero while assigning the logo belonging to a different cached hero. Please select the final hero and its logo as one pair, or resolve the logo from the final hero item.
The PR merges cleanly and its CI passes. Once these two cases are addressed, it should be ready to merge.
Thanks for this contribution. The Home caching/navigation changes look like a useful performance improvement and the PR is close to ready. I found two small edge cases that should be fixed before merging:
lastHomeDataLoadAtMs is updated before loadHomeData() succeeds. If the request fails temporarily, Home is still considered fresh and automatic resume refreshes can be skipped for six hours. Please update the timestamp only after a successful load, or restore/reset it in the failure path.
In the cached-category race fix, heroItem and heroLogoUrl are preserved independently. If Continue Watching sets a hero but has no cached logo, the base-cache path can keep that hero while assigning the logo belonging to a different cached hero. Please select the final hero and its logo as one pair, or resolve the logo from the final hero item.
The PR merges cleanly and its CI passes. Once these two cases are addressed, it should be ready to merge.
lastHomeDataLoadAtMs is now set only after the load publishes rows, so a failed load doesn't mark Home fresh. Added a homeDataLoadAttempted flag to keep the "startup owns the first load" guard the zero value used to provide, and refreshHomeDataIfStale now retries on resume when a load has been attempted but none has succeeded, rather than waiting out the TTL.
Hero and logo are now selected as one pair — a hero already set by Continue Watching keeps its own logo (resolved from the logo cache if it doesn't have one yet), and the cached hero's logo is only used when the cached hero is the one being applied.
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
area: androidChanges to the Android app or Gradle build
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.
perf(home): fix slow home load on cold start and on return
Home took ~5s to populate when returning from Watchlist/TV/Search, and intermittently
5–10s on cold start. Four causes:
1. The HTTP disk cache never ran.
OkHttpProvider.clientis built lazily and memoised,and
OkHttpProvider.init()was called fromApplication.onCreate()aftersuper.onCreate()— where Hilt's field injection already constructs Retrofit and therefore the client. So it was
always built with no app context and ran cacheless for the whole process (
cache/http_cachenever existed on device). Moved to
attachBaseContext, andinitnow falls back to the passedcontext because
getApplicationContext()returns null that early. Added a warning log when theclient builds without a context — it caught the second half of this.
2.
navigateHome()destroyed HomeViewModel.navigateTopLevelleaves Home on the back stack(
inclusive = false), but the return path re-navigated withinclusive = true, killing theNavBackStackEntrythathiltViewModel()scopes to — so the screen rebuilt from scratch everytime. Now pops back to the existing entry, with the old navigate as fallback.
popBackStackalsoclears anything above Home, which handles the stale-Details case the old comment cited.
3. A startup race discarded the cached rows (the intermittent cold start). Two coroutines
start together in
HomeViewModel.init: one restores the cached category rows from disk, theother restores Continue Watching. The categories publish was guarded by
_uiState.value.categories.isEmpty()— so whenever Continue Watching won the race and insertedits row first, the guard failed and every cached base row was silently dropped. The screen
then showed Continue Watching alone until the network load finished, which is itself held behind
a 4–5.7s startup settle delay. Whichever coroutine finished first decided the outcome, which is
why it only happened sometimes. The guard now tests for real base rows
(
hasRealBaseCategories(), mirroring whatloadHomeDataalready uses), keeps a live ContinueWatching row over the cached copy, and preserves a hero it had already set.
4. Nothing would refresh afterwards. The rebuild had been doubling as the refresh mechanism.
Added a 6h staleness check on resume (
refreshHomeDataIfStale); Continue Watching keeps its own45s refresh.
Also: a network interceptor marking unsuccessful responses
no-store, so a transient 5xx or a429 can't be cached and replayed.
Measured on device
cache/http_cacheabsent, ~550 requests per return, ~5s.After: 464 of 510 responses served from disk, 3 network calls; return is instant.
dropped, content only arriving with the network load). After the fix, 6/6 cold starts landed
at 318–458ms, publishing all 14 cached rows.