Skip to content

Fix Android thread priority #1280

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

chucknology
Copy link
Contributor

Android threads are Linux threads which differ from the POSIX standard.
Calling pthread_setschedparam does not produce the intended effect on Android.

Android threads are Linux threads which differ from the POSIX standard. Calling pthread_setschedparam does not produce the intended effect on Android.
@paulfd
Copy link
Member

paulfd commented Mar 17, 2025

@atsushieno you have more Android experience than I do, if you find some time to have a look I will defer to your judgment :) Thanks!

@atsushieno
Copy link
Contributor

atsushieno commented Mar 18, 2025

@paulfd okay, but I was not aware of this platform-specific difference either, so I'm reviewing from that stand point.

@chucknology Reviewing without actually running code so far - I agree to the change in general, but it seems that the intent of setting priority there is so that it gives "moderately higher" priority than normal thread, not "highest" (-20). The default value for config::backgroundLoaderPthreadPriority used in non-Android code is 50, so I would say -10 is better (if config is not considered).

IfUnless there is still reason to set -20 (or it does not make sense to use values like -10), I would suggest @paulfd to make another commit to make such one-liner change and merge it.

@chucknology
Copy link
Contributor Author

@atsushieno @paulfd thanks for your consideration.

My understanding is that Android's priority values map to Linux priorities (i.e. nice values) : from -20 to 19.
Some constants are defined in the android.os.Process class as follows:

public static final int THREAD_PRIORITY_URGENT_AUDIO = -19;
public static final int THREAD_PRIORITY_AUDIO = -16;
public static final int THREAD_PRIORITY_VIDEO = -10;
public static final int THREAD_PRIORITY_TOP_APP_BOOST = -10;
public static final int THREAD_PRIORITY_URGENT_DISPLAY = -8;
public static final int THREAD_PRIORITY_DISPLAY = -4;
public static final int THREAD_PRIORITY_FOREGROUND = -2;
public static final int THREAD_PRIORITY_MORE_FAVORABLE = -1;
public static final int THREAD_PRIORITY_DEFAULT = 0;
public static final int THREAD_PRIORITY_LESS_FAVORABLE = 1;
public static final int THREAD_PRIORITY_BACKGROUND = 10;
public static final int THREAD_PRIORITY_LOWEST = 19;

The comments in that class suggest that the "top app" has its main(UI) and render threads priorities "boosted" to -10, so I surmise that a lower value(higher priority) than that might be appropriate to be impactful in most cases.

In our application we uncovered and addressed significant performance bottlenecks elsewhere (unrelated to sfizz) so it was difficult to evaluate the actual benefit of this change in isolation. I can say that there have been no apparent adverse effects.

I did confirm that the implementation I've offered appears to work, remaining set to the desired priority - and the system complains appropriately when trying to set invalid values.
In any case, the existing implementation has no effect on Android so I consider this a qualitative improvement.

I'd propose setting the priority to -19 (URGENT_AUDIO) or -16 (AUDIO) as some kind of "internally consistent" choice for the Android platform.

@atsushieno
Copy link
Contributor

Ahh, thanks for the clarification for each value. That helps me understand the appropriate usages for each value.

I don't think -19 or even -16 should be given to this "background loader" thread. Recently I have investigated the Android audio framework internals (namely audioserver) and figured that THREAD_PRIORITY_AUDIO is used for audio threads in most cases, and only low latency audio API (AAudio) uses THREAD_PRIORITY_URGENT_AUDIO.

It should be noted that audio applications like YouTube Music still does NOT use AAudio, and still its background audio playback takes most priority e.g. it keeps playing music while the UI is not responsive. It applies to any media application that depends on Jetpack Media3 (ExoPlayer).

-19 and -16 are SUCH high priority values, and definitely not for background threads.

Have you verified your app behavior with any priority like THREAD_PRIORITY_FOREGROUND (-2) ? From the description of those each enum values, I assume this should be most appropriate. Or it might be insufficient until THREAD_PRIORITY_TOP_APP_BOOST (-10) for your app, but settings to this sounds like against good behavior as an Android application that prevents other apps overall.

@paulfd
Copy link
Member

paulfd commented Jul 3, 2025

I'm OK to merge if you're good with it!

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

Successfully merging this pull request may close these issues.

3 participants