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

Run worker pool at lowest thread priority #1497

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

ReillyBrogan
Copy link
Contributor

On a Linux session with Plasma desktop I was having intermittent desktop and software freezes. These freezes were always associated with RSSGuard using 100% of CPU when it does a periodic feed check (~500 feeds).

In order to resolve this we can lower the thread priority of those background threads in order to not compete with interactive system threads. Unfortunately QThread priorities don't actually work on most Linux systems as it attempts to set the process priority which is ignored with SCHED_OTHER (the default scheduler for most systems), see here for more info. Instead we need to use nice values to achieve the same effect (confusingly the Linux documentation ALSO refers to this as process priority despite the fact that it's not the same thing as the other process priority).

Since Qt doesn't have any handy helpers for this I added a setThreadPriority function that does the same thing using libc functions. While this was only tested on glibc I didn't use any GNU extensions so in theory it should work on musl-based systems as well (unsure if those are actually supported by RSSGuard or not). I also changed the scheduling hint to SCHED_BATCH which means that the scheduler will consider the thread non-interactive and potentially CPU-intensive (which gives a slight penalty to wake-up scheduling).

This code is ifdefed to only run on Linux. To achieve the same effect QThread priority levels are defined which should work on Windows, Mac, and Linux systems where RSSGuard is not started with SCHED_BATCH or SCHED_OTHER.

Also in several places I noticed that the Qt thread handle was being converted to a number for logging or other purposes. While this did result in a unique per-thread identifier on Linux systems we can get the thread ID instead which is helpful since logging output can be correlated to other system tools (like ps -lcmT (pidof rssguard) which reports scheduling type and priority for all rssguard threads). I added a helper for that and switched all uses of currentThreadId() to it (I have no idea how to retrieve the actual thread ID on Win/Mac so I kept the previous behavior for those).

@martinrotter martinrotter self-assigned this Sep 21, 2024
@martinrotter martinrotter added this to the 4.7.4 milestone Sep 21, 2024
@martinrotter martinrotter added Type-Enhancement This is request for brand new feature. Component-Core Status-Fixed Ticket is resolved. Type-Desktop-Integration Application badly integrates into used desktop environment. labels Sep 21, 2024
@martinrotter
Copy link
Owner

Hi @ReillyBrogan.

This is some amazing PR. Accepting. Will do some testing, but the code looks very good. Thanks.

@martinrotter martinrotter merged commit edad7fa into martinrotter:master Sep 21, 2024
@ReillyBrogan ReillyBrogan deleted the thread-prio branch September 22, 2024 16:34
@martinrotter
Copy link
Owner

Checked it on runtime, nice work.

Cleaned up a bit some bits of code, thanks once more.

c83ab23

@ReillyBrogan
Copy link
Contributor Author

I see you added a Qt version check for the ThreadPool priority

#if QT_VERSION >= 0x060200 // Qt >= 6.2.0

I don't think this is necessary because you already have a Qt >= 6.3.0 check in the top-level CMakeLists.txt

set(QT6_MIN_VERSION 6.3.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component-Core Status-Fixed Ticket is resolved. Type-Desktop-Integration Application badly integrates into used desktop environment. Type-Enhancement This is request for brand new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants