feat(mobile): adopt radiance memory monitor, remove Go-side memory logger#8893
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRemoves the mobile memory logger and its IPC lifecycle hooks, and updates ChangesMemory logger removal and dependency bump
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR switches mobile tunnel-process memory monitoring/logging responsibility from Lantern’s Go-side periodic runtime.ReadMemStats sampler to Radiance’s adaptive vpn/memmon monitor by bumping the radiance dependency and removing the redundant in-process Go memory logger hooks.
Changes:
- Remove the Go-side tunnel memory logger start/stop calls from
StartIPCServer/CloseIPCServer. - Delete
lantern-core/mobile/memory_logger.go(the Go runtime memstats ticker logger). - Bump
github.com/getlantern/radianceto a newer pseudo-version ingo.mod/go.sum.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lantern-core/mobile/mobile.go | Removes the lifecycle hooks that started/stopped the Go-side memory logger in the tunnel IPC server. |
| lantern-core/mobile/memory_logger.go | Deletes the periodic Go runtime memstats logger implementation. |
| go.mod | Updates the github.com/getlantern/radiance dependency version. |
| go.sum | Updates checksums for the new github.com/getlantern/radiance version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@garmr-ulfr This one looks good to me too. Do you mind fixing the merge conflicts? |
|
@garmr-ulfr, it has been tested on iOS, I did not get time to test this in Android, but I think we should test this in Android before merging it in. |
@atavism, yeah, I can do that. This is still waiting for the companion PR in radiance to be merged. For future PRs that have a radiance companion, should I keep them as drafts until the companion PR has been merged? |
Summary
Adopts the adaptive mobile memory monitor that now lives in radiance, and removes the redundant Go-side memory logger from the tunnel process.
radianceto the build that includes thevpn/memmonadaptive memory monitor (companion PR: feat(vpn): adaptive memory monitor for the mobile VPN daemon radiance#550). On iOS it reclaims proactively under jetsam pressure; on Android it runs observe-only.lantern-core/mobile/memory_logger.goand itsstartMemoryLogger()/stopMemoryLogger()calls inmobile.go. That logger sampled Go-runtime stats viaruntime.ReadMemStats(stop-the-world) on a timer in the tunnel process; radiance's monitor now covers memory observation (and reclamation) from inside the same process, without the STW pause.Summary by CodeRabbit