Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Optimize UI & GPU thread QoS for iOS #15673

Closed
wants to merge 1 commit into from
Closed

Optimize UI & GPU thread QoS for iOS #15673

wants to merge 1 commit into from

Conversation

Qxyat
Copy link
Contributor

@Qxyat Qxyat commented Jan 15, 2020

From the Instruments,we found that the UI & GPU thread run in a low priority, which may often be preempted by other thread and take less CPU time. For example:
image
The UI thread running time is 2.8s,but the preempted time is 4.18s。

According to the Apple documentation:https://developer.apple.com/library/archive/documentation/Performance/Conceptual/EnergyGuide-iOS/PrioritizeWorkWithQoS.html, the UI and GPU thread should be assigned to 'User-interactive', while the current QoS is 'Default'.

In addition, when the platform view is enabled, the GPU task runner is same as the platform task runner.So we should not create another GPU thread.

@auto-assign auto-assign bot requested a review from cbracken January 15, 2020 12:10
@Qxyat Qxyat requested a review from gaaclarke January 15, 2020 12:11
Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the intent of tweaking thread priorities but this should go in the platform view so all platforms get a chance to set these priorities for all engine managed threads.

The thing about not creating the GPU thread is inaccurate and must be left out of this patch.

@@ -682,6 +691,14 @@ - (void)setIsGpuDisabled:(BOOL)value {
_isGpuDisabled = value;
}

- (void)setupQualityOfService:(flutter::TaskRunners&)task_runners {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be done for all threads managed by the engine. This includes the IO thread as well as the worker task runners (QOS_CLASS_BACKGROUND). I suggest exposing this via the platform view creation callback. That way, all platforms are given the opportunity to set thread priorities on all threads managed by the engine.

@@ -435,12 +431,18 @@ - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI {
// TODO(amirh/chinmaygarde): remove this, and dynamically change the thread configuration.
// https://github.com/flutter/flutter/issues/23975

_threadHost = {threadLabel.UTF8String, // label
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That the GPU thread will no longer be created when there are platform views in the scene is an artifact of a previous implementation. This has been fixed #16068 and will cause issues with that line of work.

@gaaclarke
Copy link
Member

Optimally, run your app at a QoS level of utility or lower at least 90% of the time when user activity is not occurring. Use powermetrics to determine how much time is being allocated to different QoS classes.

Default | The priority level of this QoS falls between user-initiated and utility. Work that has no QoS information assigned is treated as default. The GCD global queue runs at this level. This QoS is not intended to be used to classify work.

It sounds like we'd want to do something more dynamic than just setting the QoS to User-Interactive across the board. For example, increasing the QoS of the GPU thread at vsync.

@gaaclarke
Copy link
Member

I talked to @chinmaygarde in person. This sounds fine to me after making the changes he suggested.

One point is that we could add a priority to PostTask but most of our tasks are already divided up by priority based on what thread they are on. For example, you wouldn't want to run a low priority task on the GPU thread.

@Qxyat can you post the before and after sample with your test case like you did in the PR description?

@chinmaygarde
Copy link
Member

This looks stale. I have filed flutter/flutter#63812 to track carrying this to completion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants