Skip to content
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

Android Embedding unconditionally calls NotifyLowMemory in onTrimMemory #90551

Closed
dnfield opened this issue Sep 22, 2021 · 8 comments · Fixed by flutter/engine#28891
Closed
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) customer: money (g3) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list perf: memory Performance issues related to memory perf: speed Performance issues related to (mostly rendering) speed platform-android Android applications specifically

Comments

@dnfield
Copy link
Contributor

dnfield commented Sep 22, 2021

Specifically, here:

https://github.com/flutter/engine/blob/dd28b6f1c8e68089dab16ebce9f629f309690027/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java#L781-L786

Whereas we only notify the application code if the memory level is RUNNING_LOW, which also seems like a bug.

It seems like we should also notify the application if we have RUNNING_CRITICAL or if it's the LOW/CRITICAL for not running.

It also seems like we should check at least for RUNNING_LOW/RUNNING_CRITICAL before telling the Dart VM to potentially GC. On lower end devices, I've observed this particular codepath triggering a 250ms during the critical path to first frame.

@dnfield dnfield added platform-android Android applications specifically engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) customer: money (g3) perf: memory Performance issues related to memory perf: speed Performance issues related to (mostly rendering) speed P1 High-priority issues at the top of the work list labels Sep 22, 2021
@dnfield
Copy link
Contributor Author

dnfield commented Sep 22, 2021

/cc @blasten @xster fyi

@dnfield
Copy link
Contributor Author

dnfield commented Sep 22, 2021

One thought is that if we have not yet rendered a frame, we should avoid notifying unless the memory is *CRITICAL.

@dnfield
Copy link
Contributor Author

dnfield commented Sep 22, 2021

Added a little instrumentation and this may be appropriate in the specific case of the trace I'm seeing, even if our level detection is a little suspicious.

@blasten
Copy link

blasten commented Sep 22, 2021

right. Do you know where the 250ms are spent? is it in performDeferredCleanup ? https://github.com/flutter/engine/blob/825c409164c1c78cef1e9e3ee3f47b066c1294e7/shell/common/rasterizer.cc#L130

0ms for the LRU cache seems very low, isn't it?

@dnfield
Copy link
Contributor Author

dnfield commented Sep 23, 2021

It's ~250ms in a dart GC. e.g.

image

Where that CollectOldGeneration is taking 256 CPU/321ms wall time.

@dnfield
Copy link
Contributor Author

dnfield commented Sep 25, 2021

Here's another example though, where the app was definitely not in the background. At the end of the first Choreographer#doFrame, Android calls trimMemory, which immediately triggers a GC when we'd want to be more focused on producing the first frame:

image

@dnfield
Copy link
Contributor Author

dnfield commented Sep 25, 2021

(This time the trim memory was almost certainly a RUNNING_LOW rather than a RUNNING_CRITICAL or UI_HIDDEN)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) customer: money (g3) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list perf: memory Performance issues related to memory perf: speed Performance issues related to (mostly rendering) speed platform-android Android applications specifically
Projects
None yet
3 participants