-
-
Notifications
You must be signed in to change notification settings - Fork 347
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
fix high CPU usage #1481 #1484
fix high CPU usage #1481 #1484
Changes from 8 commits
d058db6
5d33d3a
c68b298
53b3fe3
43d44d2
4093f57
8e6a973
c6f656f
0595767
1bed0b3
50eaed6
41a3883
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -21,7 +21,7 @@ func retryAxCallUntilTimeout_(_ group: DispatchGroup?, _ timeoutInSeconds: Doubl | |||
} catch { | ||||
let timePassedInSeconds = Double(DispatchTime.now().uptimeNanoseconds - startTime.uptimeNanoseconds) / 1_000_000_000 | ||||
if timePassedInSeconds < timeoutInSeconds { | ||||
BackgroundWork.axCallsQueue.asyncAfter(deadline: .now() + .milliseconds(10)) { | ||||
BackgroundWork.axCallsQueue.asyncAfter(deadline: .now() + .milliseconds(1000)) { | ||||
retryAxCallUntilTimeout_(group, timeoutInSeconds, fn, startTime) | ||||
} | ||||
} | ||||
|
@@ -247,6 +247,80 @@ extension AXUIElement { | |||
func performAction(_ action: String) { | ||||
AXUIElementPerformAction(self, action as CFString) | ||||
} | ||||
|
||||
private static let ignoredBundleIDs = Set([ | ||||
"com.apple.dashboard", | ||||
"com.apple.loginwindow", | ||||
"com.apple.notificationcenterui", | ||||
"com.apple.wifi.WiFiAgent", | ||||
"com.apple.Spotlight", | ||||
"com.apple.systemuiserver", | ||||
"com.apple.dock", | ||||
"com.apple.AirPlayUIAgent", | ||||
"com.apple.dock.extra", | ||||
"com.apple.PowerChime", | ||||
"com.apple.WebKit.Networking", | ||||
"com.apple.WebKit.WebContent", | ||||
"com.apple.WebKit.GPU", | ||||
"com.apple.FollowUpUI", | ||||
"com.apple.controlcenter", | ||||
"com.apple.SoftwareUpdateNotificationManager", | ||||
"com.apple.TextInputMenuAgent", | ||||
"com.apple.TextInputSwitcher" | ||||
]) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's a good idea to hardcode specific I think we should find more flags or properties of apps that give strong hints that they will not spawn windows. Also retrying less often than 10ms is already a large workaround the CPU issue |
||||
|
||||
/** | ||||
* Returns a Bool indicating whether or not the application will have windows. | ||||
* | ||||
* @return true if the application will have windows and false otherwise. | ||||
*/ | ||||
static func isManageable(_ runningApp: NSRunningApplication) -> Bool { | ||||
guard let bundleIdentifier = runningApp.bundleIdentifier else { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have cases like the android emulator where the app has no alt-tab-macos/src/logic/Applications.swift Line 127 in 0259057
|
||||
return false | ||||
} | ||||
if case .prohibited = runningApp.activationPolicy { | ||||
return false | ||||
} | ||||
if AXUIElement.ignoredBundleIDs.contains(bundleIdentifier) { | ||||
return false | ||||
} | ||||
if isAgent(runningApp) { | ||||
return false | ||||
} | ||||
return true | ||||
} | ||||
|
||||
// LSBackgroundOnly (Boolean - macOS) specifies whether this app runs only in the background. | ||||
// If this key exists and is set to YES, Launch Services runs the app in the background only. | ||||
// You can use this key to create faceless background apps. | ||||
// You should also use this key if your app uses higher-level frameworks that connect to the window server, but are not intended to be visible to users. | ||||
// Background apps must be compiled as Mach-O executables. This option is not available for CFM apps. | ||||
|
||||
// LSUIElement (Boolean - macOS) specifies whether the app runs as an agent app. | ||||
// If this key is set to YES, Launch Services runs the app as an agent app. | ||||
// Agent apps do not appear in the Dock or in the Force Quit window. | ||||
// Although they typically run as background apps, they can come to the foreground to present a user interface if desired. | ||||
// A click on a window belonging to an agent app brings that app forward to handle events. | ||||
/** | ||||
* Returns a Bool indicating whether or not the application is an agent. | ||||
* | ||||
* @return true if the application is an agent and false otherwise. | ||||
*/ | ||||
static func isAgent(_ runningApp: NSRunningApplication) -> Bool { | ||||
guard let bundle = Bundle.init(url: runningApp.bundleURL!) else { | ||||
return false | ||||
} | ||||
guard let bundleInfoDictionary = bundle.infoDictionary else { | ||||
return false | ||||
} | ||||
if bundleInfoDictionary["LSBackgroundOnly"] != nil { | ||||
return true | ||||
} | ||||
if bundleInfoDictionary["LSUIElement"] != nil { | ||||
return true | ||||
} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I remember correctly, checking for these is redundant with I may be wrong though. I would be nice to confirm that by printing it for a few apps and confirming that in some cases these can disagree. That would mean we should check all of those. If they always agree, then this is not helping |
||||
return false | ||||
} | ||||
} | ||||
|
||||
enum AxError: Error { | ||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -158,20 +158,23 @@ class Application: NSObject { | |||
|
||||
private func observeEvents() { | ||||
guard let axObserver = axObserver else { return } | ||||
for notification in Application.notifications(runningApplication) { | ||||
retryAxCallUntilTimeout { [weak self] in | ||||
guard let self = self else { return } | ||||
try self.axUiElement!.subscribeToNotification(axObserver, notification, { | ||||
DispatchQueue.main.async { [weak self] in | ||||
guard let self = self else { return } | ||||
// some apps have `isFinishedLaunching == true` but are actually not finished, and will return .cannotComplete | ||||
// we consider them ready when the first subscription succeeds, and list their windows again at that point | ||||
if !self.isReallyFinishedLaunching { | ||||
self.isReallyFinishedLaunching = true | ||||
self.observeNewWindows() | ||||
// we only need to subscribe to those apps which will have windows | ||||
if AXUIElement.isManageable(runningApplication) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check is a bit late. Better not even create the alt-tab-macos/src/logic/Applications.swift Line 104 in 0259057
You could add your new filtering logic there. That being said, see the comment in the method I linked. Some apps actually change the value of their There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the suggestion. I found a compensation method to solve this problem. The initial technical attempts have been made, and I'll implement the code for this later. I'm a bit busy these two weeks. |
||||
for notification in Application.notifications(runningApplication) { | ||||
retryAxCallUntilTimeout { [weak self] in | ||||
guard let self = self else { return } | ||||
try self.axUiElement!.subscribeToNotification(axObserver, notification, { | ||||
DispatchQueue.main.async { [weak self] in | ||||
guard let self = self else { return } | ||||
// some apps have `isFinishedLaunching == true` but are actually not finished, and will return .cannotComplete | ||||
// we consider them ready when the first subscription succeeds, and list their windows again at that point | ||||
if !self.isReallyFinishedLaunching { | ||||
self.isReallyFinishedLaunching = true | ||||
self.observeNewWindows() | ||||
} | ||||
} | ||||
} | ||||
}, self.runningApplication) | ||||
}, self.runningApplication) | ||||
} | ||||
} | ||||
} | ||||
CFRunLoopAddSource(BackgroundWork.accessibilityEventsThread.runLoop, AXObserverGetRunLoopSource(axObserver), .defaultMode) | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a value of
250ms
may be a better comprise: 10ms is very aggressive for a responsive UX but at the cost of CPU. 1s is easy on the CPU, but a bit laggy. I think250ms
is similar to human delay in processing changes on screen. So maybe a better compromise. See https://humanbenchmark.com/tests/reactiontime