Android embedding does not call Shell::NotifyLowMemoryWarning when there is memory pressure

[dnfield]

It just sends the system channel notification, which the engine does not listen for specially.

We could fix this by either making the engine (i.e. in engine.cc) listen for this channel message like others and notify the shell when it hears the message (and then forward it along), or we could expose the shell call through the JNI bindings so that we can do it the way it's done on iOS and in the embedder API.

If we update the engine.cc for this, we should take out the iOS/embedder portions. If we update Android for it, we just have to make sure we're consistent.

One other possible issue that comes to mind is that we may want to try to call Dart_NotifyLowMemory after we send the channel message, on the off chance that the Dart code has enough time to actually free some caches to let the garbage collector pick up even more garbage.

References:

New embedding:

https://github.com/flutter/engine/blob/a6435210acaee2fcd4f0f049032c47c043cfd51f/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java#L636 https://github.com/flutter/engine/blob/a6435210acaee2fcd4f0f049032c47c043cfd51f/shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java#L654

Old embedding:

https://github.com/flutter/engine/blob/a6435210acaee2fcd4f0f049032c47c043cfd51f/shell/platform/android/io/flutter/view/FlutterView.java#L323

Compare with iOS: https://github.com/flutter/engine/blob/a6435210acaee2fcd4f0f049032c47c043cfd51f/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm#L747-L750

Compare with Embedder API: https://github.com/flutter/engine/blob/a6435210acaee2fcd4f0f049032c47c043cfd51f/shell/platform/embedder/embedder.cc#L1840-L1853

@xster @matthew-carroll @jason-simmons @chinmaygarde for input.

[dnfield]

I'm inclined to say it should just be exposed through the JNI to avoid messing with other embedders and perhaps unexpectedly changing behavior.

[xster]

I'd love us to stop using the channel snooping pattern. The indirection is very opaque and hard to trace mentally. We should just add a JNI for it.

Collecting after the sending the Flutter hint is a good idea. I suppose it's a bit tricky because it's asynchronous (on iOS too) with not just the channel call return but with its side effect on the next frame. One existing mechanism we can piggyback on is to hook onto the renderer's next frame callback when we send SystemChannel.memoryPressure, then Dart GC in the next frame callback.

[dnfield]

Yeah, I strongly doubt we'll do much better with the order. And unfortunately, we can't sit around waiting for a channel response that might come too late.

[xster]

ya, it's a different problem magnitude. Can you create a separate optimization issue for that?

[matthew-carroll]

I think I'd take a different tack on this issue.

First, I think JNI is something that we should aim to eliminate eventually. It's a pain to work with, not many people understand it, and it's far more difficult to validate than something like a channel.

I can't speak for how the engine currently registers with channels. I also understand that channels currently have some limitations with regard to synchronicity.

However, I think this issue is one in a category of issues: things that could be done via channels but are done via JNI, instead, because our channel solution isn't necessarily robust enough. In other words, are channels fundamentally incapable of reasonably supporting this behavior? Or is it that channels in their current state make it more difficult and therefore we go with JNI as a quick and easy solution?

I tried my best to move things out of JNI when I did the refactor. It would be unfortunate to start promoting that particular interface again.

I would also tend to think that if the engine code generally represents the portable, platform agnostic subsystem, it would make more sense for every platform integration to leverage channels for their platform messages rather than have every platform use a distinct technology-specific interface to talk to the engine.

[xster]

We're kinda talking about the same thing right? https://cs.opensource.google/flutter/engine/+/master:shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java;l=72;drc=025e2d82dda54af7f33a0d511bde47ec835593b1

It's just whether we (serialize and) multiplex many methods into one JNI method or we keep them flat.

[dnfield]

It would be better for this to be as low latency as possible - we don't necessarily have much time to get the work done here, and don't want to eat into that time with serialization and deserialization of the message channel.

[matthew-carroll]

@xster not really. The difference between a singular implementation detail that never changes, and which maintainers never see, is functionally very different than an interface that maintainers will see, and expand, and be tasked with testing. You're correct that the technical mechanism is the same, but I would not categorize those 2 approaches as equivalent for cognitive overhead, extensibility, or testability.

@dnfield is there not a conceivable extension to how channels work that would allow us to achieve that low latency? Could channels offer a capability that solves that problem?

[dnfield]

Unfortunately, I don't think so. The platform channels are inherently async and can get backed up behind each other. Then there's also serialization/deserialization overhead - we could in theory have a message that is very cheap to deserialize (e.g. just a single bit as true or false), but we still run into potential issues where the channel is crowded with other messages and by the time we process those it's too late.

[dnfield]

PR was reverted

[github-actions[bot]]

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.