-
Notifications
You must be signed in to change notification settings - Fork 254
feat_: implement low memory mode with garbage collection logging #6549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a low memory mode by invoking garbage collection and logging pre‑ and post‑GC memory statistics.
- Introduces a thread-safe mechanism to throttle GC execution to once every 30 seconds.
- Adds functions SwitchToLowMemoryMode and gc to encapsulate the GC trigger and logging logic.
Comments suppressed due to low confidence (2)
mobile/status.go:2460
- [nitpick] The function name 'gc' might be confused with the standard library function runtime.GC. Consider renaming it to a more descriptive name like 'runGarbageCollection' to avoid ambiguity.
gc()
mobile/status.go:2492
- The calculation for 'released_mb' uses (after.HeapReleased - before.HeapReleased). Verify that this reflects the intended meaning; if the intention is to log the memory released, the subtraction order may need to be reversed.
zap.Int64("released_mb", int64(after.HeapReleased-before.HeapReleased)/1024/1024),
Jenkins BuildsClick to see older builds (47)
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6549 +/- ##
===========================================
- Coverage 60.44% 60.41% -0.04%
===========================================
Files 848 848
Lines 104995 105027 +32
===========================================
- Hits 63466 63450 -16
- Misses 33953 33976 +23
- Partials 7576 7601 +25
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@qfrank is this draft, or should we review? |
af2d7c9
to
a5be759
Compare
ready to review :) @igor-sirotin |
a5be759
to
1cbcf0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious how often and under which circumstances you trigger this endpoint from mobile. Have you considered using GOMEMLIMIT instead?
It depends on onTrimMemory|onLowMemory to trigger this endpoint.
Not yet, what's the suitable mem value for limiting? Any idea? @osmaczko |
Makes sense, that's great android provides such callback already.
Good question. This needs to be evaluated from the mobile side. The appropriate value should come from observing the app’s runtime behavior under varying conditions, particularly by tracking metrics like average If |
I tried to fix populate-db tool, generated lots of data, args like:
and then profile the behavior of status-go, do you think if it's worth continue? @osmaczko |
would first observe how |
6f8401f
to
e174de4
Compare
logging output example:
close relate issue 6547
relate mobile PR