[engine] support combined UI/Platform thread for iOS/Android.

[jonahwilliams]

Experimentally support merging UI and platform thread on Android/iOS. This works by changing the shell holder to set the UI thread to the platform thread. Several shell APIs that post messages from the platform to ui thread were changed to use RunNowOrPostTask to immediately call the UI task if the threads are the same.

Experimentally, this seems to work reasonably well if there are no platform views. On Android with TLHC it works fine either way, while iOS currently takes a big performance hit.

This can be opted into via a Plist:

	<key>FLTEnableMergedPlatformUIThread</key>
	<true/>

Or via AndroidManifest.xml:

 <meta-data
            android:name="io.flutter.embedding.android.EnableMergedPlatformUIThread"
            android:value="true" />

flutter/flutter#150525

[chinmaygarde]

I'd check the headerdoc in platform_view.h to be sure. But I think this can be called on any thread by the embedder. I also don't think this is used at all on non-Fuchsia platforms. And even there, I think we moved away from deferred libraries split into units. But, the code to service such a path may still be there.

[jonahwilliams]

It doesn't say. This isn't for fuchsia, its for the android app bundle/deferred library support.

[jonahwilliams]

it looks like this is probably only called from the platform thread, but I'm not sure if we have any real tests here so I'll leave it as is.

[chinmaygarde]

I'd prefer we start with the tester shells first. It is easy to go merge the threads but there is no going back because of plugins.

This change itself LGTM.

[jonahwilliams]

The tester shells are already running in a combined thead mode (unless run with --force-multithreading).

It is easy to go merge the threads but there is no going back because of plugins.

Adding an experimental option that we don't advertise is not a one way door. Anyone that ships code that depends on this behavior can be broken

[knopp]

I love this! I'll try to get this working on macOS with the resize synchronizer, but I'm not sure how much time I'm going to have this week (FlutterCon).

Btw. the io.flutter.1.ui thread still gets created, just sitting there empty.

[knopp]

I have a very fragile and buggy but working macOS proof of concept working. The idea is to short-circuit macOS vsync waiter during resizing to fire immediately, and then to modify the vsync waiter machinery in the engine so that if the vsync callback is called from UI thread and frame_start_time is in past, it will trigger the frame immediately instead.

[jonahwilliams]

Btw. the io.flutter.1.ui thread still gets created, just sitting there empty.

Should be fixed now.

[knopp]

I've a WIP PR for macos here. Some remaining open questions there, mostly regarding the microtask queue.

[jonahwilliams]

@chinmaygarde do you intend to review this patch?

[knopp]

This looks good to me. I am a bit unsure though why is the fml::MessageLoop initialized on platform thread for both iOS and Android. This predates platform isolates so I'm wondering if the reason is raster thread merging?

Having fml::MessageLoop available for platform thread means that no changes are necessary to run Dart code on platform thread, but same thing will not work for embedder API based embedders, so I'm wondering if we could be more consistent here (does not need to happen in this PR).

[chinmaygarde]

Adding an experimental option that we don't advertise is not a one way door. Anyone that ships code that depends on this behavior can be broken.

Isn't it though? It will fracture the plugin ecosystem into plugins that assume calls are made directly on the platform thread (the stuff using FFI) vs those that post messages back and forth. There is kinda no going back if enough plugins start assuming this.

Flags end up getting used and we don't have an experiments mechanism like in Chrome that timeboxes and backs out experiments that don't pan out.

The mechanics of moving the UI thread to the platform are fairly straightforward. But my understanding was that while we were intending to do this, there were open questions around performance and we didn't know how much of a performance hit we were willing to tolerate in heavy applications alike the 1P apps and stuff like Wonderous with the platform views.

So, I think this should come after those questions are answered.

[jonahwilliams]

I simply can't agree that an experimental flag presents an opportunity for ecosystem breakage. I agree that we should good answers for the question of performance before we make this the default behavior, but opting in for testing and experimentation is well within the realm of changes like platform isolates, wide gamut, impeller, et cetera.

[chinmaygarde]

Fair points.

I know this isn't possible now but my dream would be to have something like the origin trials in Chrome. We'd like to be able to safely gauge the impact of specific experiments while ensuring that users don't build their businesses on experimental stuff (perhaps unknowingly). The origin trials also have a way of backing out experiments that don't pan out (either because they are bad ideas or there weren't the resources to carry them to completion).

I'm absolutely not averse to adding additional experimental flags. Its just that this one is unique in that:

• Perhaps unknowingly, a user may depend on a plugin that makes FFI calls on the platform thread. They might need to disable the flag say because of performance issues and now their app is thread-unsafe. There is no way for us to detect such races or offer support.
• The stuff you mentioned (platform isolates, wide gamut, impeller, ...) can be backed out cleanly via the single flag and we either have fallbacks in place or clear error conditions. The notion that building plugins and toying with the flag "may introduce undefined behavior due to data races that we cannot detect easily" muddies the waters with this one.

But, I see your point. We need to be able to experiment with such things and we don't have a mechanism to do this cleanly. I'd highly appreciate buy in from from someone on the ecosystem side for this one. Just to calm my nerves about this one if nothing.

The mechanics of this patch LGTM.

[knopp]

If the plugin assumes merged UI and platform thread, it would mean that the plugin imposes the flag on the end user, which I don't really like. For example with super_drag_and_drop and super_clipboard, I make quite a few assumptions about threading (I routinely need to pause platform thread to while getting answer from UI thread). I plan to support merged threads as soon as it lands on all platforms, but as an addition to the default threading model, and not exclusively.

I think we need to communicate this clearly that if plugins make threading assumptions, they need to support both models for the time being, and never force the flag on end user.

[reidbaker]

I am not sure you can force plugin authors to support both models. Exposing to plugins which model is being used then asking nicely might be your only option. If you let plugin authors detect the threading model and let the apps set the threading model then plugin authors can choose how to handle the difference. Plugins that dont care can Ignore the change, some will support both, some can throw exceptions or can use the thread model to enable or disable a subset of features.

[jonahwilliams]

To be clear, this flag is for our own experimentation.

There is absoltely no plan to make this a configurable option for developers. Either we find a means to merge the platform and ui threads, in general, and make that the only default behavior, or we don't.

I'm not currently aware of any circumstances where moving from un merged to merged thread model will break any plugins, but this is something that I hope to discover this quarter. (I could certainly see the opposite breaking plugins, but that won't be a configurable option)

[knopp]

This will absolutely cause super_drag_and_drop to deadlock, since it needs to respond to synchronous platform callbacks and it does it by carefully blocking the platform thread until it has response from UI thread. But I also think this is a very uncommon scenario.

[jonahwilliams]

Assuming we had a solution to the other thread merging problems, I'd like us to handle that with some like "Starting from (Future) version 3.XX, threads will be merged, if you do X/Y/Z instead do A/B/C" instead of letting folks opt in or out.