fix(swift-sdk): drain the persister's autorelease pool per row - #4414
fix(swift-sdk): drain the persister's autorelease pool per row#4414romchornyi wants to merge 1 commit into
Conversation
The persister callbacks arrive on Rust-owned tokio worker threads, and such a thread has no autorelease pool: the drain is normally done by a run loop or by GCD's own worker wrapper, and `serialQueue.sync` can run the block straight on the calling thread. Nothing in this file — 8600 lines and 38 C callbacks — opened one. That matters because SwiftData is Core Data underneath. Resolving a managed object hands back autoreleased Foundation objects, and `-[_NSCoreManagedObjectID URIRepresentation]` alone allocates an `NSURL`, an `NSPathStore2` and two `CFString`s. `resolveInputOutpoint` calls into that per input, per transaction, per block. On a wallet with thousands of relevant blocks the app reached a 3 GB footprint and was killed before its initial scan finished. A heap dump at that point: 7,512,707 live `NSURL`s, 7,512,604 `NSPathStore2`s, 15,052,753 `CFString`s, 33M live allocations, 183 MB of autorelease pool pages. `malloc_history` put every one of them under `persistWalletChangesetCallback` → `upsertTransaction` → `resolveInputOutpoint` → `URIRepresentation`. There are two pools, and the inner one is the fix. A pool in `onQueue` alone still drains only when the whole changeset is done, and a large wallet's changesets are big enough that the interim peak was still millions of objects — measured, not assumed: it moved the count from 7.5M to 6.5M. The per-row pools in `applyAccountChangeset` keep the peak flat regardless of batch size. After: the same wallet syncs to completion — 8582 filters matched, 3269 relevant blocks, 5095 transactions — at a 496 MB footprint with 1k–18k live `NSURL`s and no growth.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe persistence handler now scopes autoreleased allocations during serial queue operations and individual transaction and UTXO processing iterations in ChangesPersistence memory management
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change adds per-row autorelease-pool draining to reduce memory growth during large wallet scans without changing persistence behavior; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
✅ Final review complete — no blockers (commit 0382551) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The exact-head diff adds autorelease pools around serialized persistence work and each high-volume transaction/UTXO row without changing queue confinement, FFI pointer lifetimes, operation ordering, or save boundaries. No in-scope correctness, interoperability, or performance regressions were identified.
Source: reviewers codex general (gpt-5.6-sol) and codex ffi-engineer (gpt-5.6-sol); final verifier codex (gpt-5.6-sol). Orchestration only, not reviewer evidence: openclaw-agent/cliproxy/gpt-5.6-sol.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
3 participants
Issue being fixed or feature implemented
The app reaches a ~16 GB physical footprint during a large wallet's initial scan and is killed before the scan finishes. The logs stay quiet the whole time, which is what made this hard to attribute.
The persister callbacks arrive on Rust-owned tokio worker threads, and such a thread has no autorelease pool — the drain is normally done by a run loop or by GCD's own worker wrapper, and
serialQueue.synccan run the block straight on the calling thread. Nothing in this file — 8200 lines and 38 C callbacks — opened one.That matters because SwiftData is Core Data underneath. Resolving a managed object hands back autoreleased Foundation objects, and
-[_NSCoreManagedObjectID URIRepresentation]alone allocates anNSURL, anNSPathStore2and twoCFStrings.resolveInputOutpointreaches that per input, per transaction, per block.Heap dump at 3 GB, taken from the running app:
NSURLNSPathStore2CFString@autoreleasepool contentpages (183 MB)33 M live allocations,
MALLOC_SMALLat 3.1 GB.malloc_historyput every one of them underpersistWalletChangesetCallback→applyAccountChangeset→upsertTransaction→resolveInputOutpoint→URIRepresentation.What was done?
Two pools, and the inner one is the fix.
A pool in
onQueuealone still drains only when the whole changeset is done, and a large wallet's changesets are big enough that the interim peak was still millions of objects. That was measured, not assumed: it moved the liveNSURLcount from 7.5 M to 6.5 M — i.e. almost nothing. The per-row pools inapplyAccountChangeset(transactions, added UTXOs, spent UTXOs, InstantSend locks) keep the peak flat regardless of batch size.How Has This Been Tested?
Measured on the reproducing wallet, same device and same restore-from-seed scenario, before and after:
NSURLgrowth=+0.0 MB/sxcodebuild test -scheme SwiftDashSDK— 352 tests, 0 failures, on the branch this was developed on. CI covers it here.Breaking Changes
None.
autoreleasepoolwrappers around existing call sites; no behaviour, ordering or atomicity change — nosave()was added or moved.Checklist
For repository code-owners and collaborators only
Summary by CodeRabbit