AppKit APIs that block the main queue cause UI to hang on macOS

jmatth commented 2 years ago

Details

Full description

It appears there are some operations within AppKit on macOS that can noticeably delay or entirely block the main queue. This manifests in Flutter as jank or the UI completely hanging. The two relevant actions I have found are interacting with the menu bar (jank) and showing a native popup menu (hang).

Running in profile mode shows that the hang occurs in FlutterCompositorPresentLayers. Debugging the engine eventually leads to FlutterResizeSynchronizer.requestCommit. This function attempts to dispatch some work on the main queue and wait for it to complete. If the main queue is blocked, this causes the UI to hang.

To test I changed the requestCommit to run its work on a background queue with the highest QoS of USER_INTERACTIVE (see below). This resolved the issue and allowed the UI to run smoothly while popup menus were opened and the menu bar was interacted with. ~However, it did seem to have a performance impact as UI frametimes went from ~0.2ms to ~0.4ms in the profiler while showing a single loader animation~. Edit: Upon further investigation it's not clear whether this has a significant performance impact. My initial results are invalid because I did not realize the engine compiles only for x64 by default, so the 0.2ms slowdown could be attributed to running through Rosetta. I haven't found a consistent way to compare the performance before and after this change. Someone familiar with profiling the engine performance would likely have better luck.

I also attempted to fix the issue on the plugin side by triggering the popup menu in a background queue. This is apparently disallowed as exceptions were thrown from inside of AppKit every time I tried. It is my understanding that this is not an uncommon restriction for Apple to place on certain APIs in their platforms.

Modified `requestCommit` code

--- a/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm
+++ b/shell/platform/darwin/macos/framework/Source/FlutterResizeSynchronizer.mm
@@ -121,7 +121,7 @@
     // No resize, schedule commit on platform thread and wait until either done
     // or interrupted by incoming BeginResize
     [_delegate resizeSynchronizerFlush:self];
-    dispatch_async(dispatch_get_main_queue(), [self, cookie = _cookie] {
+    dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), [self, cookie = _cookie] {
       std::unique_lock<std::mutex> lock(_mutex);
       if (cookie == _cookie) {
         if (_delegate) {

Steps to reproduce

Execute flutter run -d macos on the code sample on a macOS machine
Click the Summon menu button
Click elsewhere to dismiss the contextual menu that appeared
With the app still focused, click View (or any other item) in the menu bar
Click outside the open menu to close it

Expected results:

The app animations should continue running while the popup menu is open
The app animations should run smoothly throughout the interaction with the menu bar

Actual results:

The app animations stop entirely while the popup menu is open
The app animations jank when the menu bar menu is closed

Code sample

As some of the reproduction requires a plugin package to trigger the macOS context menu, I have created a git repo with the full reproduction here: [jmatth/flutter_macos_hang_repro][repro_repo]. The contents of `lib/main.dart` from that repo are below. It should work as long as the `contextual_menu` plugin is installed. ```dart import 'package:contextual_menu/contextual_menu.dart' as cm; import 'package:flutter/material.dart'; void main() { runApp(const MyApp()); } class MyApp extends StatelessWidget { const MyApp({Key? key}) : super(key: key); // This widget is the root of your application. @override Widget build(BuildContext context) { return MaterialApp( title: 'Main thread block repro', theme: ThemeData( primarySwatch: Colors.blue, ), home: const MyHomePage(title: 'Flutter Demo Home Page'), ); } } class MyHomePage extends StatelessWidget { const MyHomePage({ super.key, required this.title, }); final String title; @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: Text(title), ), body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ // const Placeholder(), const CircularProgressIndicator(), const SizedBox(height: 20), ElevatedButton( onPressed: () { cm.popUpContextualMenu(cm.Menu( items: [ cm.MenuItem( label: 'Item 1', ), ], )); }, child: const Text( 'Summon menu', ), ), ], ), ), ); } } ```

Video recording:

As I already traced to root cause and it is not related to constrained system resources, I used the screen record function on macOS instead of recording the monitor with my phone. If you feel it is still necessary to record with an external device please let me know.

Also sorry for the .json.txt files for the timeline traces, github didn't like .json files for some reason.

Hang and jank

https://user-images.githubusercontent.com/1316184/170315748-73b363aa-c7cc-493d-b2b3-869092908457.mov

Timeline trace: jank-timeline.json.txt

Fixed with custom engine build

https://user-images.githubusercontent.com/1316184/170316394-6f7332de-3f70-4967-b432-ab3b3ff75165.mov

Timeline trace: custom_engine.json.txt

Target Platform: macOS Target OS version/browser: 12.4 Devices: Physical 14 inch M1 Pro Macbook Pro

Logs

``` Warning: You are using these overridden dependencies: ! contextual_menu 0.1.1 from git https://github.com/jmatth/contextual_menu.git at 88ff51 ! menu_base 0.1.0 from git https://github.com/jmatth/menu_base.git at 2b24c8 Running "flutter pub get" in flutter_macos_hang_repro... 417ms Analyzing flutter_macos_hang_repro... No issues found! (ran in 3.0s) ``` ``` Doctor summary (to see all details, run flutter doctor -v): [✓] Flutter (Channel master, 3.1.0-0.0.pre.902, on macOS 12.4 21F79 darwin-arm, locale en-US) [✓] Android toolchain - develop for Android devices (Android SDK version 32.1.0-rc1) [✓] Xcode - develop for iOS and macOS (Xcode 13.3.1) [✓] Chrome - develop for the web [✓] Android Studio (version 2021.2) [✓] Android Studio (version 2021.1) [✓] VS Code (version 1.67.2) [✓] Connected device (2 available) [✓] HTTP Host Availability • No issues found! ```

huycozy commented 2 years ago

Hi @jmatth. Thanks for filing the issue. I can reproduce this issue on latest stable and master channels.

Due to contextual_menu is a 3rd party plugin rather than Flutter's plugin, I've replaced context menu with a CupertinoContextMenu for demonstration.

Demo

https://user-images.githubusercontent.com/104349824/170443010-07e88cdc-816b-45be-8411-1572f281e57b.mp4

Sample code

```dart import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({Key? key}) : super(key: key); static const String _title = 'Flutter Code Sample'; @override Widget build(BuildContext context) { return const MaterialApp( title: _title, home: MyStatelessWidget(), ); } } class MyStatelessWidget extends StatelessWidget { const MyStatelessWidget({Key? key}) : super(key: key); @override Widget build(BuildContext context) { return Scaffold( body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ const CircularProgressIndicator(), SizedBox(height: 100,), SizedBox( width: 100, height: 100, child: CupertinoContextMenu( actions: [ CupertinoContextMenuAction( child: const Text('Action one'), onPressed: () { Navigator.pop(context); }, ), CupertinoContextMenuAction( child: const Text('Action two'), onPressed: () { Navigator.pop(context); }, ), ], child: Container( color: Colors.red, ), ), ), ], ), ), ); } } ```

DevTools log data

[dart_devtools_2022_5_26-1653549984389000.json.zip](https://github.com/flutter/flutter/files/8777284/dart_devtools_2022_5_26-1653549984389000.json.zip)

flutter doctor -v

```bash [✓] Flutter (Channel stable, 3.0.1, on macOS 12.2.1 21D62 darwin-x64, locale en-VN) • Flutter version 3.0.1 at /Users/huynq/Documents/GitHub/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision fb57da5f94 (8 hours ago), 2022-05-19 15:50:29 -0700 • Engine revision caaafc5604 • Dart version 2.17.1 • DevTools version 2.12.2 [✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0) • Android SDK at /Users/huynq/Library/Android/sdk • Platform android-32, build-tools 31.0.0 • ANDROID_HOME = /Users/huynq/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 13.3) • Xcode at /Applications/Xcode.app/Contents/Developer • CocoaPods version 1.11.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2021.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840) [✓] Android Studio (version 4.1) • Android Studio at /Users/huynq/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/201.7042882/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6915495) [✓] Android Studio • Android Studio at /Users/huynq/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-1/203.7185775/Android Studio Preview.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 11.0.8+10-b944.6842174) [✓] IntelliJ IDEA Community Edition (version 2020.3.3) • IntelliJ at /Applications/IntelliJ IDEA CE.app • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart [✓] VS Code (version 1.67.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.40.0 [✓] Connected device (3 available) • iPhone 13 (mobile) • 2526BC1A-435D-4B08-B99C-44B928F2517B • ios • com.apple.CoreSimulator.SimRuntime.iOS-15-4 (simulator) • macOS (desktop) • macos • darwin-x64 • macOS 12.2.1 21D62 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 101.0.4951.64 [✓] HTTP Host Availability • All required HTTP hosts are available • No issues found! ``` ```bash [✓] Flutter (Channel master, 3.1.0-0.0.pre.931, on macOS 12.2.1 21D62 darwin-x64, locale en-VN) • Flutter version 3.1.0-0.0.pre.931 at /Users/huynq/Documents/GitHub/flutter_master • Upstream repository https://github.com/flutter/flutter.git • Framework revision 20a9f1d8d7 (2 hours ago), 2022-05-25 18:43:11 -0700 • Engine revision 480610ca4c • Dart version 2.18.0 (build 2.18.0-149.0.dev) • DevTools version 2.13.1 [✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0) • Android SDK at /Users/huynq/Library/Android/sdk • Platform android-32, build-tools 31.0.0 • ANDROID_HOME = /Users/huynq/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 13.3) • Xcode at /Applications/Xcode.app/Contents/Developer • CocoaPods version 1.11.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2021.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840) [✓] Android Studio (version 4.1) • Android Studio at /Users/huynq/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/201.7042882/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6915495) [!] Android Studio • Android Studio at /Users/huynq/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-1/203.7185775/Android Studio Preview.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart ✗ Unable to find bundled Java version. • Try updating or re-installing Android Studio. [✓] IntelliJ IDEA Community Edition (version 2020.3.3) • IntelliJ at /Applications/IntelliJ IDEA CE.app • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart [✓] VS Code (version 1.67.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.40.0 [✓] Connected device (3 available) • iPhone (mobile) • d9a94afe2b649fef56ba0bfeb052f0f2a7dae95e • ios • iOS 15.5 19F77 • macOS (desktop) • macos • darwin-x64 • macOS 12.2.1 21D62 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 101.0.4951.64 [✓] HTTP Host Availability • All required HTTP hosts are available ! Doctor found issues in 1 category. ```

Refer this to meta-issue: https://github.com/flutter/flutter/issues/74833

jmatth commented 2 years ago

Great, thanks.

In my testing and the video you added CupertinoContextMenu doesn't cause the same issue because it's rendered in Flutter rather than calling any AppKit APIs. To completely block rendering something needs to call NSMenu.popUp, and as far as I can tell no built-in Flutter plugins do that.

It's not that I'm attached to contextual_menu in particular, the issue is that this makes it impossible to write plugins for macOS that use a certain set of AppKit APIs without causing the app to freeze. If a separate issue needs to be filed to address this as a bug in the plugins system rather than / in addition to a performance bug please let me know and I'll do so. I suspect that any fix for the jank caused by interacting with the menu bar will also fix the popUp issue though.

jmatth commented 2 years ago

I did a bit more testing and realized my initial estimate of ~0.2ms slowdown is probably invalid. I didn't realize that the engine compiles to x64 by default so testing with the default engine was running natively, while the version with my changes was running through Rosetta, which might account for the slowdown.

I tried recompiling to arm64 and testing again but frametimes aren't consistent enough to conclusively say whether there's a significant performance impact. I'm going to look into running the benchmarks I see in the engine repo with and without my changes. Assuming those show no major change it seems like running the resize callback on the USER_INTERACTIVE queue could be a viable fix.

gspencergoog commented 2 years ago

cc @cbracken

cbracken commented 2 years ago

The TL;DR is that we advise against blocking the platform thread (the macOS main thread) in the Engine architecture wiki. That thread is responsible for keyboard events, pointer events, etc. so blocking that thread may cause those events to be lost. You can see some previous discussion of this on this issue: https://github.com/flutter/flutter/issues/22024#issuecomment-465289341

That said, we almost certainly shouldn't be blocking existing Dart-driven animation; that's a bug. Thanks for reporting this.

JosefWN commented 1 year ago

Possibly related, although to my knowledge I'm not using AppKit: I'm having very long FlutterCompositorPresentLayers on Metal (that is, both iOS and macOS ARM/Intel), even on high-end devices. This occurs when a layer in flutter_map is rebuilt using a StreamBuilder. These frames can be several times longer than my frame budget, and SceneDisplayLag is significant.

I can cause similar rebuilds by interacting with the map, in which case I'm well within the frame budget. The StreamBuilder is rebuilt when data is received over the network and sent over the stream. If the StreamBuilder rebuild occurs as the user is interacting with the map I'm thinking it might cause jank. All these slow frames are also cluttering the dev tools, as I'm receiving data pretty frequently.

Even on low-end Windows machines the performance is more predictable and within frame budget. This is recurring so I'm assuming it's not related to shader compilation jank.

Tried compiling with Impeller on iOS just for fun (Flutter 3.3.0), but both performance and features were too lacking for the app to function properly.

EDIT: No jank, just skipped frames, but that is also not ideal since the layer doesn't update when the user interacts with the map.

MegatronKing commented 1 year ago

Mark! I have the same issue.

knopp commented 1 year ago

We're calling [NSMenu popUpMenuPositioningItem:] in NativeShell all the time without any issues. I think the problem in your case is that you're blocking the main dispatch queue.

Unlike CFRunLoop, dispatch queues are not reentrant. When a queue callback is blocked, no other dispatch queue callback is delivered. Flutter uses dispatch queue to schedule platform channel calls as well as screen updates.

When you call [NSMenu popup] from platform channel callback, you will block the main dispatch queue so none of these are delivered.

You should be able to reschedule the call to the main run loop directly, (i.e. using [NSRunLoop performBlock:] and call [NSMenu popup] from this block. That way you'll free the dispatch queue. [NSMenu popup] will run the even loop, ensuring the main displatch queue is drained properly.

jmatth commented 1 year ago

Thanks @knopp! You were absolutely right that moving the call to the main run loop fixed the issue.

For anyone else in this thread with the same issue, here's what worked for me in Swift:

// Wrapper class to pass closures to Objective-C APIs that take selectors taken from https://stackoverflow.com/a/36983811/1988017
final class Action: NSObject {

    private let _action: () -> ()

    init(action: @escaping () -> ()) {
        _action = action
        super.init()
    }

    @objc func action() {
        _action()
    }
}

//...

let action = Action(action: {
  self.menu?.popUp(...)
})
RunLoop.current.perform(
  #selector(action.action),
  target: action,
  argument: nil,
  order: 0,
  modes: [RunLoop.Mode.common]
)

Once Flutter completes its ongoing migration to only support macOS 10.13+, it should be possible to simplify this code to use a more idiomatic version of perform that was introduced in macOS 10.12:

RunLoop.current.perform {
  menu.popUp(...)
}

This only addresses plugins calling NSMenu.popUp and similar APIs, and not the jank observed when closing menus in the menubar on macOS. I need to take some time to poke around that part of the engine and see if the same or a similar fix can be implemented there.

flutter-triage-bot[bot] commented 1 month ago

The triaged-desktop label is irrelevant if there is no team-desktop label or fyi-desktop label.

flutter / flutter