Skip to content

perf(sync): scope remote save discovery by installed ROMs - #270

Open
brendannoble wants to merge 5 commits into
rommapp:mainfrom
brendannoble:perf/bulk-save-discovery
Open

perf(sync): scope remote save discovery by installed ROMs#270
brendannoble wants to merge 5 commits into
rommapp:mainfrom
brendannoble:perf/bulk-save-discovery

Conversation

@brendannoble

@brendannoble brendannoble commented Aug 28, 2026

Copy link
Copy Markdown

What changed

  • scope remote save discovery to the uncovered ROMs installed on the device using RomM's repeated rom_ids query parameter
  • sort and deduplicate positive ROM IDs, then request them sequentially in groups of at most 500
  • keep the existing 16 MiB and 10,000-record limits on each response, allowing a complete discovery to safely exceed either limit across multiple batches
  • detect servers that ignore rom_ids, discard the suspect response, and perform one independently validated broad request
  • preserve the existing per-ROM fallback with at most four requests in flight when scoped or broad discovery fails
  • preserve existing sync-item and save-write orchestration; discovery itself performs no save writes

rommapp/romm#4304 has merged and provides server-side rom_ids filtering. Older RomM versions remain correct through the compatibility behavior.

Why

Remote save discovery previously made one request per uncovered ROM. RomM #4304 provides a bounded way to request saves for the ROMs a device actually has.

Request scaling on a supporting RomM server:

  • 1 to 500 uncovered ROMs: N requests to 1
  • more than 500 uncovered ROMs: N requests to ceil(N / 500)
  • 20,000 uncovered ROMs: 20,000 requests to 40
  • missing device ID or failed bulk discovery: existing N-request fallback, with at most four in flight

Decoding remains linear in requested ROM IDs plus returned save records. Batches are sequential to keep server load and client memory predictable.

Safety and failure behavior

  • accept only HTTP 200 or 204
  • reject malformed, truncated, trailing, interrupted, oversized, or over-limit responses without exposing partial results
  • aggregate batch results privately and discard them all if a later batch fails
  • filter exact requested ROM IDs and never silently truncate IDs or save records
  • treat the 16 MiB and 10,000-record limits as per-response guards, not whole-library limits
  • retry once without rom_ids when an out-of-scope record shows that the server ignored the new parameter
  • omit only the affected ROM when an individual fallback request fails
  • make no discovery request when there are no uncovered ROMs

Validation

  • go test ./...
  • go test -race ./romm ./sync
  • go vet ./...
  • changed packages pass staticcheck; repository-wide output has only existing unrelated findings
  • changed Go files pass gofmt and git diff --check
  • Linux Docker builds completed for amd64, arm64, 386, and arm/v7
  • tests cover repeated query values, exact 500/500/1 batching, positive-ID filtering and deduplication, per-response limits, later-batch atomic failure, legacy ignored-scope handling, and bounded fallback

Device validation

The save flow was tested on a ROCKNIX RG34XX-SP:

  • synchronized an existing save and confirmed it loaded correctly in-game
  • created a new save and synchronized it back to RomM
  • confirmed RomM retained both save versions
  • observed no missing saves or unintended changes

That device run exercised the legacy compatibility path because its RomM server did not include #4304. The now-merged wire contract is covered by Grout HTTP integration tests that send repeated rom_ids values in 500-ID batches. RomM merge commit c3241a7 passed its MariaDB and PostgreSQL endpoint suites, and go test ./romm ./sync -count=1 passes on this branch.

Stream and locally filter device-scoped save records, preserve authoritative empty responses, and use bounded per-ROM fallback only after bulk errors. Add safety limits, diagnostics, and regression coverage.
@brendannoble

Copy link
Copy Markdown
Author

RomM PR rommapp/romm#4304 addresses the server-side part of this performance problem by adding rom_ids scoping with a maximum of 500 IDs per request. I am updating this PR to consume that contract directly: one scoped request for up to 500 ROMs and sequential 500-ID batches for larger libraries, while retaining per-response safety limits and the existing bounded fallback for older or failing servers. This keeps the Grout change focused and avoids requiring the broader pagination proposal.

@brendannoble
brendannoble marked this pull request as draft August 29, 2026 20:25
@brendannoble brendannoble changed the title perf(sync): use one bulk request for remote save discovery perf(sync): scope remote save discovery by installed ROMs Aug 29, 2026
@brendannoble
brendannoble marked this pull request as ready for review August 29, 2026 21:39
@brendannoble

Copy link
Copy Markdown
Author

RomM PR rommapp/romm#4304 has merged into master at c3241a7. I compared the final merged contract with this branch: RomM accepts repeated rom_ids parameters, caps each request at 500 IDs, and deduplicates the scope. This branch uses the same repeated form and sequential 500-ID batches. RomM's merge commit passed its MariaDB and PostgreSQL pytest jobs, build, e2e, typecheck, and test jobs. Fresh Grout verification also passes: go test ./romm ./sync -count=1.

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