perf(auth): build the token store lazily and warm it off the main thread - #362
Merged
Conversation
Creating the encrypted token store and decrypting the active session both ran on the thread startKoin runs on, because TokenDataStore did the work in its field initializer and its init block. Build the store on first use instead, and let an eager TokenCacheWarmer perform the cache load on the IO dispatcher. The synchronous load stays as the fallback, which is what makes this safe: if the warm loses the race to a reader, the load happens on the caller's thread exactly as before. A lost race costs the previous behaviour, not a wrong bearer. TokenDataStore publishes the built store through a single @volatile reference rather than a store plus a "created" flag, so no reader can observe "built" before the store is published, read that as "the keystore gave up", and route a login into the in-memory fallback where it dies at process death. warmTokenCache takes stateMutex so it cannot interleave with a mutation, and ensureTokenLoaded reads through without publishing, leaving the warm the only writer of the cache fields outside a mutation. emitSessionExpired resolves the active account through storage while the cache is cold, since comparing against an unpopulated field would suppress a real expiry. Also defers CommonSessionCacheCleaner's cache root behind a function: it is a constructor dependency of AuthRepositoryImpl, so resolving the root eagerly put a disk read on the main thread whenever NavHostViewModel was built. TokenManager is unchanged.
Contributor
Android debug APKArtifact:
|
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
1 participant
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.
Summary
Cold start built the encrypted token store and decrypted the active session on the
main thread, inside
startKoin. The store is now created on first use, and an eagerTokenCacheWarmerperforms the decrypt on the IO dispatcher instead.The synchronous read behind
isAuthenticated/getAccessTokenstays, now readingthrough to storage rather than seeding the cache. A reader arriving before the warm
lands therefore gets the same value it would have before — paying a decrypt per call
until the warm publishes, rather than risking a stale bearer.
Changes
Token store
TokenDataStore(Android) creates itsEncryptedSharedPreferenceson first use,published through a single
@Volatilereference. One reference rather than a storeplus a
createdflag: two fields let a reader observe "built" before the store ispublished, read that as "the keystore gave up", and route a login into the in-memory
fallback, where it is lost at process death.
CommonTokenDataStore.warmTokenCache()— idempotent, takesstateMutex, so a warmcannot interleave with a mutation;
tokenInitializedbecomes@Volatile.ensureTokenLoaded()reads through to storage and publishes nothing, leaving thewarm as the only writer of the cache fields outside a mutation.
emitSessionExpiredresolves the active account through storage when the cache isnot yet warm, instead of comparing against an unpopulated field. That comparison is
a secure-storage read on the caller's thread during the pre-warm window.
TokenCacheWarmer, an eager Koin single that performs the load rather than awaitingone triggered elsewhere.
async warm (keychain rather than keystore) with the same unwarmed read-through
window.
ServerUrlKeychainFallback.readServerUrlstays synchronous and unwarmed bydesign.
Session cache cleaner
CommonSessionCacheCleanertakescacheRoot: () -> String, evaluated on theteardown paths that clear caches (logout and last-account removal). It is a
constructor dependency of
AuthRepositoryImpl, so an eagerly resolved root put adisk read on the main thread when
NavHostViewModelwas first built.NSCachesDirectorynow throws insideclearFileCaches, where it is caught and logged, rather than at DI resolution.Caches go uncleared with a warning instead of failing the first frame.
TokenManageris unchanged;isAuthenticatedandgetAccessTokenare kept.Verification
Unit tests, including a new
CommonTokenDataStoreWarmTestcovering constructiontouching storage zero times, warm idempotency under concurrency, a warm ordered
against a login, and the read-through fallback.
Android emulator (API 37), against the parent commit through an identical rig, five
cold starts per state:
main, 15/15 runscreateEncryptedPrefscacheDirreadam start -W, median)Logged-in, logged-out, expired-session cold starts, an account switch and a sign-out
all behave as before. With the warm suppressed for the length of a sign-in, the first
write still persists to the encrypted store and a logged-in launch is carried by the
synchronous fallback alone.
Not verified: physical hardware, and iOS beyond compilation.
Notes
With the keystore work off the main thread, an unrelated pre-existing main-thread disk
read becomes visible at cold start: slf4j's
ServiceLoaderscanning the apk during Ktorclient creation. Not introduced here; worth its own issue.