fix(): Performance Enhancements - PICs storage/retrieval and overall library upgrades - #1721
Open
phobos665 wants to merge 7 commits into
Open
fix(): Performance Enhancements - PICs storage/retrieval and overall library upgrades#1721phobos665 wants to merge 7 commits into
phobos665 wants to merge 7 commits into
Conversation
…ndering steam apps. More to come.
…s as well as sorting/filtering etc.
Contributor
📝 WalkthroughWalkthroughThe change adds Room projections and chunked DAO operations for Steam PICS synchronization, updates license and package processing with debouncing and timeout handling, configures database executors and indexes, and adds cached library filtering, pagination, Steam size calculations, and credential-aware counts. ChangesSteam PICS synchronization
Library filtering and pagination
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SteamCallback
participant SteamService
participant SteamLicenseDao
participant SteamAppDao
participant PicsApi
SteamCallback->>SteamService: Submit license list
SteamService->>SteamLicenseDao: Read stubs and persist license changes
SteamService->>SteamAppDao: Read metadata and update package assignments
SteamService->>PicsApi: Request product info and access tokens
PicsApi-->>SteamService: Return PICS data
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt`:
- Around line 232-238: Update updatePackageIdForApps to step through appIds
using SQLITE_MAX_VARS - 1, reserving one bind variable for packageId while
keeping the existing chunking and update behavior unchanged.
In `@app/src/main/java/app/gamenative/di/DatabaseModule.kt`:
- Around line 42-47: Replace the unbounded executor used by setQueryExecutor in
the Room database configuration with a bounded thread pool that limits query
concurrency during PICS sync bursts. Leave setTransactionExecutor unchanged, and
preserve the existing database setup and executor behavior otherwise.
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 4204-4208: Update the changedLicenses predicate in the license
diff to also mark a package changed when the existing
SteamLicenseStub.accessToken differs from pkg.accessToken, while preserving the
existing lastChangeNumber comparison and null-stub handling so rotated tokens
are requeued for PICS.
- Around line 4473-4479: Update the timeout handling in the app/package PICS
collectors around the CancellationException path and the license callback that
queues package PICS so timed-out batches are requeued with bounded retries and
backoff instead of being dropped. Preserve ensureActive behavior for genuine
coroutine cancellation, and ensure retry state prevents unbounded loops while
allowing failed metadata requests to be attempted later.
- Around line 281-284: Update the session teardown logic in
SteamService.clearDatabase() to cancel and null out licenseListDebounceJob,
clear pendingLicenseCallback, reset licenses, and remove cachedLicenseDao
alongside licenseDao. Ensure delayed onLicenseList() processing cannot
repopulate license tokens after logout.
- Around line 4153-4163: Move the in-memory assignment in the license callback
flow so `licenses = callback.licenseList` occurs immediately after receiving the
callback and before `licenseListDebounceJob?.cancel()` and the debounce launch.
Keep the debounced database/PICS work unchanged, using the updated `licenses`
value for immediate download authentication.
- Around line 4260-4269: The cached-license replacement around
cachedLicenseDao.deleteAll() and the chunked insert loop is visible to readers
between transactions. Make the delete and all chunk inserts execute within a
single database transaction so getLicensesFromDb() observes either the previous
complete snapshot or the new complete snapshot, while preserving the existing
chunking behavior.
In `@app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt`:
- Around line 663-670: Move the filterJob assignment into the existing
synchronized(this) block in onFilterApps, alongside cancellation and job
creation, so all reads and writes are protected by the same monitor. Remove the
post-block .also assignment while preserving the current cancellation and launch
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8190d7d-c9dc-4e50-96f3-2b2b098ebff6
📒 Files selected for processing (8)
app/src/main/java/app/gamenative/data/SteamAppPicsMeta.ktapp/src/main/java/app/gamenative/data/SteamLicenseForPics.ktapp/src/main/java/app/gamenative/data/SteamLicenseStub.ktapp/src/main/java/app/gamenative/db/dao/SteamAppDao.ktapp/src/main/java/app/gamenative/db/dao/SteamLicenseDao.ktapp/src/main/java/app/gamenative/di/DatabaseModule.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
Author
|
Will go through AI feedback and see what's relevant. |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/service/SteamService.kt (1)
4603-4674: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winDeduplicate
queuebefore callingpicsGetAccessTokens.queuecan contain the same app ID multiple times when an app appears in more than one package, and duplicate IDs can make the access-token request fail on some SteamKit versions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/service/SteamService.kt` around lines 4603 - 4674, Deduplicate the app IDs in queue before passing them to steamApps.picsGetAccessTokens and before constructing the subsequent PICSRequest chunks. Preserve the existing processing and logging behavior, using the deduplicated collection for request size reporting and product-info requests.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/SteamService.kt (1)
4407-4501: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated PICS retry/backoff loop.
Both collectors carry the same
while (picsAttempt <= MAX_PICS_BATCH_RETRIES)+ cancellation-vs-timeout-disambiguation shell almost verbatim. The file already cross-references the two copies in comments, which is itself a sign they need to stay in sync — a shared helper removes that maintenance burden.♻️ Sketch of a shared retry helper
private suspend fun <T> runPicsBatchWithRetry( label: String, itemCount: Int, request: suspend () -> T, ): T? { var attempt = 0 while (attempt <= MAX_PICS_BATCH_RETRIES) { try { return request() } catch (e: CancellationException) { ensureActive() attempt++ if (attempt > MAX_PICS_BATCH_RETRIES) { Timber.w("PICS $label timed out for $itemCount item(s); max retries exceeded, dropping batch") } else { Timber.w("PICS $label timed out for $itemCount item(s); retry $attempt of $MAX_PICS_BATCH_RETRIES") delay(attempt.toLong() * PICS_RETRY_BACKOFF_MS) } } catch (e: AsyncJobFailedException) { Timber.w("Could not get PICS $label $e") return null } } return null }Both collectors would then call this with their specific
job.await()body.Also applies to: 4517-4694
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/service/SteamService.kt` around lines 4407 - 4501, Extract the duplicated retry and backoff logic from both PICS collectors into a shared suspend helper such as runPicsBatchWithRetry, parameterized by the operation label, item count, and suspend request. Preserve cancellation disambiguation via ensureActive, retry limits and backoff, AsyncJobFailedException handling, and nullable failure behavior; update both collectors to invoke the helper around their job.await() request bodies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 4603-4674: Deduplicate the app IDs in queue before passing them to
steamApps.picsGetAccessTokens and before constructing the subsequent PICSRequest
chunks. Preserve the existing processing and logging behavior, using the
deduplicated collection for request size reporting and product-info requests.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 4407-4501: Extract the duplicated retry and backoff logic from
both PICS collectors into a shared suspend helper such as runPicsBatchWithRetry,
parameterized by the operation label, item count, and suspend request. Preserve
cancellation disambiguation via ensureActive, retry limits and backoff,
AsyncJobFailedException handling, and nullable failure behavior; update both
collectors to invoke the helper around their job.await() request bodies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9f6b5e8-7e70-4d62-aa17-5ff0d2dbaf5d
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.ktapp/src/main/java/app/gamenative/di/DatabaseModule.ktapp/src/main/java/app/gamenative/service/SteamService.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt
This was referenced Jul 15, 2026
christopher-s
added a commit
to christopher-s/GameNative
that referenced
this pull request
Jul 31, 2026
…k, with dropped-batch self-healing Ports utkarshdalal#1721 (supersedes utkarshdalal#1719): - Slim Room projections (SteamAppPicsMeta, SteamLicenseStub, SteamLicenseForPics) so PICS collectors stop deserializing full blobs - onLicenseList: immediate in-memory licenses, 5s debounce for callback bursts, diff-based license writes, PICS queueing only for new/changed packages, atomic cached-license snapshot transaction - Batched DB reads in PICS app/package collectors (no per-app full-row reads); IN() queries chunked under SQLite's 999-var limit - 30s PICS job timeouts + bounded retries with backoff - Room: bounded query executor, two new steam_app indexes - LibraryViewModel: pagination fast path over the cached filtered list, cancellable filter/page jobs, precomputed sort keys, single installed- branch query, depot-size cache, hoisted credential checks Fork addition after quad review (code/perf/architect/KISS): PICS batches that exhaust retries are stashed and re-queued on the next license sync (fires on every login), restoring the self-healing the old requeue- everything behavior provided; otherwise a dropped batch would leave permanently stale package metadata.
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.
Description
This is a PR that composes of a few key changes mostly to fix issues regarding CPU & RAM blockages as well as DB deadlocks.
Overview of changes:
Recording
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Speeds up Steam library sync and browsing by slimming queries, batching writes, and bounding Room threads. Adds safer PICS timeouts, retries, and cancellation handling to prevent CPU/RAM spikes and DB deadlocks.
Performance
SteamAppPicsMeta,SteamLicenseStub,SteamLicenseForPics) to avoid loading large blobs.package_idupdates by target package; resolved winners in memory using ownership/expiry.steam_appindexes ondlc_for_app_idandpackage_id; set a bounded fixed thread pool for queries and a cached pool for transactions.Stability
Written for commit d62837f. Summary will update on new commits.
Summary by CodeRabbit
Performance Improvements
Bug Fixes