Skip to content

fix(): PICs improvements and reduce memory pressure - #1719

Open
phobos665 wants to merge 2 commits into
utkarshdalal:masterfrom
phobos665:fix/pics-performance
Open

fix(): PICs improvements and reduce memory pressure#1719
phobos665 wants to merge 2 commits into
utkarshdalal:masterfrom
phobos665:fix/pics-performance

Conversation

@phobos665

@phobos665 phobos665 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

  • Reduced size of grabbing licences from the DB via a smaller licence grab
  • Match chunking of the queries to the batch size for pics to avoid overflow

Recording

Type of Change

  • Bug fix
  • Performance / stability improvement
  • Compatibility improvements
  • Other (requires prior approval)

Checklist

  • If I have access to #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.
  • This change aligns with the current project scope (core functionality, stability, or performance). If not, it has been explicitly approved beforehand.
  • I have attached a recording of the change.
  • I have read and agree to the contribution guidelines in CONTRIBUTING.md.

Summary by cubic

Cuts memory use and stabilizes PICS requests by fetching only the needed license fields and matching chunk sizes to the PICS batch size.

  • Performance
    • Added SteamLicenseForPics and getLicensesForPics() to select only packageId and access_token.
    • Aligned chunking to MAX_PICS_BUFFER for caching, inserts, and PICS requests (replaces hardcoded 500).
    • Switched PICS request generation to use getLicensesForPics() and added clearer progress logs to reduce memory pressure on large libraries.

Written for commit be8a4b1. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Performance Improvements
    • Improved Steam license processing by handling records in controlled batches.
    • Reduced loaded data for image-related license handling by only fetching required fields.
  • Reliability
    • Added clearer progress logging around Steam license insert operations and refresh behavior.

…what is required for PICRequest processing to reduce memory pressure
@phobos665
phobos665 requested a review from utkarshdalal as a code owner July 15, 2026 09:18
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Steam license handling now uses a slim Room projection for PICS processing, batches license inserts with MAX_PICS_BUFFER, and refreshes projected licenses after persistence.

Changes

Steam license PICS flow

Layer / File(s) Summary
License projection contract and query
app/src/main/java/app/gamenative/data/SteamLicenseForPics.kt, app/src/main/java/app/gamenative/db/dao/SteamLicenseDao.kt
Adds a Room-mapped SteamLicenseForPics model and updates getLicensesForPics to select only packageId and access_token.
License list batching and refresh
app/src/main/java/app/gamenative/service/SteamService.kt
Uses MAX_PICS_BUFFER for license insertion batches, updates insertion logging, and refreshes data through getLicensesForPics.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: utkarshdalal

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title matches the change set: PICS-related performance improvements and reduced memory pressure.
Description check ✅ Passed The description follows the template and includes the required sections, with clear summary, type, and checklist items.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/SteamService.kt (1)

4180-4182: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: Chunking is unnecessary for Room's @Insert.

While chunking cachedLicenseDao.insertAll above effectively saves memory by avoiding a massive list of serialized JSON strings all at once, chunking licenseDao.insertAll here doesn't yield the same memory benefit because licensesToAdd is already fully materialized in memory.

Additionally, Room's @Insert for collections handles large lists gracefully without hitting SQLite's 999 parameter limit. It does this by compiling a single INSERT statement and executing it in a loop for each entity within the transaction, rather than expanding the entire list into a single massive query (unlike IN (:list) queries).

You can safely pass the entire list to insertAll to avoid allocating intermediate chunk lists.

♻️ Proposed refactor
-                    licensesToAdd.chunked(MAX_PICS_BUFFER).forEach { chunk ->
-                        licenseDao.insertAll(chunk)
-                    }
+                    licenseDao.insertAll(licensesToAdd)
🤖 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 4180 -
4182, Update the license insertion flow around licenseDao.insertAll to pass
licensesToAdd directly instead of iterating over chunked intermediate lists.
Keep the existing insertion behavior unchanged while removing the unnecessary
chunk allocation.
🤖 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.

Nitpick comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 4180-4182: Update the license insertion flow around
licenseDao.insertAll to pass licensesToAdd directly instead of iterating over
chunked intermediate lists. Keep the existing insertion behavior unchanged while
removing the unnecessary chunk allocation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f23cb5d5-333d-4143-82e7-2316dea16be3

📥 Commits

Reviewing files that changed from the base of the PR and between 23f61c3 and 5e7d451.

📒 Files selected for processing (3)
  • app/src/main/java/app/gamenative/data/SteamLicenseForPics.kt
  • app/src/main/java/app/gamenative/db/dao/SteamLicenseDao.kt
  • app/src/main/java/app/gamenative/service/SteamService.kt

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread app/src/main/java/app/gamenative/service/SteamService.kt Outdated
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
@phobos665

Copy link
Copy Markdown
Contributor Author

Is now encompassed by: #1721

But feel free to merge this first to reduce the overall size of that PR.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant