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

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

Open
jmatth opened this issue May 25, 2022 · 9 comments
Open

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

jmatth opened this issue May 25, 2022 · 9 comments
Labels
a: animation Animation APIs a: desktop Running on desktop c: performance Relates to speed or footprint issues (see "perf:" labels) d: devtools DevTools related - suite of performance and debugging tools found in release: 3.0 Found to occur in 3.0 found in release: 3.1 Found to occur in 3.1 has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list perf: speed Performance issues related to (mostly rendering) speed platform-mac Building on or for macOS specifically team-desktop Owned by Desktop platforms team triaged-desktop Triaged by Desktop platforms team

Comments

@jmatth
Copy link
Contributor

jmatth commented May 25, 2022

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

  1. Execute flutter run -d macos on the code sample on a macOS machine
  2. Click the Summon menu button
  3. Click elsewhere to dismiss the contextual menu that appeared
  4. With the app still focused, click View (or any other item) in the menu bar
  5. 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. The contents of lib/main.dart
from that repo are below. It should work as long as the contextual_menu plugin
is installed.

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: <Widget>[
            // 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

Screen.Recording.2022-05-25.at.12.21.03.mov

Timeline trace:
jank-timeline.json.txt

Fixed with custom engine build

Screen.Recording.2022-05-25.at.12.22.59.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

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!
@jmatth jmatth added the from: performance template Issues created via a performance issue template label May 25, 2022
@danagbemava-nc danagbemava-nc removed the from: performance template Issues created via a performance issue template label May 26, 2022
@huycozy huycozy added the in triage Presently being triaged by the triage team label May 26, 2022
@huycozy
Copy link
Member

huycozy commented May 26, 2022

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
104638.mp4
Sample code
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: <Widget>[
                  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

flutter doctor -v
[✓] 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!
[✓] 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: #74833

@huycozy huycozy added a: animation Animation APIs c: performance Relates to speed or footprint issues (see "perf:" labels) platform-mac Building on or for macOS specifically a: desktop Running on desktop d: devtools DevTools related - suite of performance and debugging tools perf: speed Performance issues related to (mostly rendering) speed has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.0 Found to occur in 3.0 found in release: 3.1 Found to occur in 3.1 and removed in triage Presently being triaged by the triage team labels May 26, 2022
@jmatth
Copy link
Contributor Author

jmatth commented May 26, 2022

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
Copy link
Contributor Author

jmatth commented May 26, 2022

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
Copy link
Contributor

cc @cbracken

@gspencergoog gspencergoog added the P2 Important issues not at the top of the work list label May 26, 2022
@cbracken
Copy link
Member

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: #22024 (comment)

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

@JosefWN
Copy link

JosefWN commented Sep 3, 2022

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
Copy link

Mark! I have the same issue.

@knopp
Copy link
Member

knopp commented Jan 7, 2023

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
Copy link
Contributor Author

jmatth commented Jan 9, 2023

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 flutter-triage-bot bot added team-desktop Owned by Desktop platforms team triaged-desktop Triaged by Desktop platforms team labels Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: animation Animation APIs a: desktop Running on desktop c: performance Relates to speed or footprint issues (see "perf:" labels) d: devtools DevTools related - suite of performance and debugging tools found in release: 3.0 Found to occur in 3.0 found in release: 3.1 Found to occur in 3.1 has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list perf: speed Performance issues related to (mostly rendering) speed platform-mac Building on or for macOS specifically team-desktop Owned by Desktop platforms team triaged-desktop Triaged by Desktop platforms team
Projects
None yet
8 participants