dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.52k stars 338 forks source link

High cpu usage when `show_age_threshold = -1`. #1163

Closed fwsmit closed 1 year ago

fwsmit commented 1 year ago

Described by @bakkeby:

As per the comment in the dunst config file the show_age_threshold can be set to -1 to disable the display of the age of a message.

In queues_get_next_datachange if there are no messages that are timing out (i.e. they are permanent until the user interacts with them) then the function will return -1.

https://github.com/dunst-project/dunst/blob/464076d4054811bc7261f2ed684b5713b18fdf51/src/queues.c#L589

Now in the run function we do not take into account that the return value can be -1 indicating that there is no need to do any further updates.

https://github.com/dunst-project/dunst/blob/464076d4054811bc7261f2ed684b5713b18fdf51/src/dunst.c#L95-L115

The solution can be as simple as bailing if that is the case:

         if (active) {
                 gint64 timeout_at = queues_get_next_datachange(now);
+                if (timeout_at == -1)
+                        return G_SOURCE_REMOVE;
                 // Previous computations may have taken time, update `now`
                 // This might mean that `timeout_at` is now before `now`, so
                 // we have to make sure that `sleep` is still positive.
                 now = time_monotonic_now();
                 gint64 sleep = timeout_at - now;
                 sleep = MAX(sleep, 1000); // Sleep at least 1ms

                 LOG_D("Sleeping for %li ms", sleep/1000);

                 if (sleep >= 0) {
                         if (reason == 0 || next_timeout < now || timeout_at < next_timeout) {
                                 if (next_timeout != 0) {
                                         g_source_remove(next_timeout_id);
                                 }
                                 next_timeout_id = g_timeout_add(sleep/1000, run, NULL);
                                 next_timeout = timeout_at;
                         }
                 }
         }

(there are other ways to refactor the code to the same effect of course)

Example test scenario is to:

Expected behaviour without the change above:

Expected behaviour with the change above:

fwsmit commented 1 year ago

@bakkeby Your fix seems to work. Could you submit it or something similar as a PR?

bakkeby commented 1 year ago

Sure, I'll raise one.

fwsmit commented 1 year ago

Fixed with #1164

myme commented 1 year ago

I've been consistently struggling with this issue for a while (and sorry to say without putting effort into solving it). I built from master after #1168 yesterday and have seen no trace of CPU spin ups since. Great work! :+1: