Avoid GCing aggressively during startup, and forward trim messages beyond low to application #28891
Conversation
…yond low to application
|
In local traces of customer: money's startup time, this substantially improves startup performance on a low memory 4 core device. |
| // not at least running critical, we should avoid delaying the frame for | ||
| // an overly aggressive GC. | ||
| boolean trim = | ||
| isFirstFrameRendered ? level >= TRIM_MEMORY_RUNNING_LOW : level > TRIM_MEMORY_RUNNING_LOW; |
There was a problem hiding this comment.
>= for the last part of the ternary too so readers don't have to go look for what the next level is?
| isFirstFrameRendered ? level >= TRIM_MEMORY_RUNNING_LOW : level > TRIM_MEMORY_RUNNING_LOW; | ||
| if (trim) { | ||
| flutterEngine.getDartExecutor().notifyLowMemoryWarning(); | ||
| flutterEngine.getSystemChannel().sendMemoryPressureWarning(); |
There was a problem hiding this comment.
so we're running it more now? From being only in TRIM_MEMORY_RUNNING_LOW to all levels above TRIM_MEMORY_RUNNING_LOW?
There was a problem hiding this comment.
Correct.
I'm not sure why the previous behavior was the way it was, but in this case we'll now run it when the app goes to background. I can move this to a separate change if you think that's worthwhile.
What's bad right now is that I suspect you can get RUNNING_CRITICAL and never hit RUNNING_LOW, or you might go to background and get one of the background related ones without ever hitting a RUNNING_* one.
There was a problem hiding this comment.
Sorry, to clarify:
We're running flutterEngine.getDartExecutor().notifyLowMemoryWarning() less now. This is the bigger culprit since it almost always will force an expensive GC.
We're running flutterEngine.getSystemChannel.sendMemoryPressureWarning() more now, which is less serious because it does not by itself trigger GC, just triggers a platform message to the Dart application to do cleanup.
There was a problem hiding this comment.
right. I think one PR is ok. Just to check on potential expectations, is that with this PR, we might increase jank (more GC events going into the framework), and we might decrease OOM (since we're not missing some signals now). Is that right?
There was a problem hiding this comment.
That sounds right to me
|
I've fixed the failing test so that it now makes sure to mark a frame as rendered for the second half. |
|
do we know what is causing this signal? Is this vendor specific? |
|
@blasten - I see it happening on a Google specific phone at the end of I don't know if it's vendor specific. |
|
...and do we know if any of this can be attributed to how different Flutter components allocate memory during this phase? |
|
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/ActivityThread.java;l=1629;bpv=0;bpt=1 is where this gets scheduled - you can see there that trimMemory gets pushed to the end of the next choreographer frame. I suppose we could try to do something similar, so that we respond to |
| public void onFlutterUiDisplayed() { | ||
| host.onFlutterUiDisplayed(); | ||
| isFlutterUiDisplayed = true; | ||
| isFirstFrameRendered = true; |
There was a problem hiding this comment.
nit: can isFlutterUiDisplayed be used instead?
There was a problem hiding this comment.
That flips to false if the ui goes away
|
@xster the tests I've been able to run internally do not show regressions, but due to some blockers currently I haven't been able to run very many. Can we land this for now and keep an eye out as a patch that is worth looking at should regressions arise? |
…yond low to application (flutter#28891)
…sages beyond low to application (flutter/engine#28891)
…sages beyond low to application (flutter/engine#28891)
…yond low to application (flutter#28891)
Fixes flutter/flutter#90551
This PR does the following:
Dart_NotifyLowMemorywhenever we get a trim signal, useTRIM_LEVEL_RUNNING_CRITICALand above before a first frame andTRIM_LEVEL_RUNNING_LOWafter as the cutoffs.TRIM_LEVEL_RUNNING_LOW, call it for all trim levels aboveRUNNING_LOW.This can slightly raise memory usage during application initialization, but only until a first frame is rendered.