GNOME Bugzilla – Bug 573718
Change g_timeout_adds to g_timeout_add_seconds to consolidate wakeups
Last modified: 2009-03-06 22:52:11 UTC
Inspired from https://wiki.ubuntu.com/SavingTheWorld Patch below, All are equivalent except they should consolidate wakeups except for the PING_TIMEOUT_DELAY which I increased from 2.25 seconds to 3.
Created attachment 129831 [details] [review] wakeups should be less evil
None of these are "repeated" timeouts that matter for wakeups; they are one-shot timeouts. Changing to add_seconds is probably bad in the ping case, where it is a user experience issue how long that timeout lasts. add_seconds both makes the length nondeterministic (since it has to snap to next even second) and changes it from 2.25 to 3.
Created attachment 129912 [details] [review] Use seconds instead Removed the ping one, as long as the others don't need the accuracy I still think switching to seconds make sense. Thanks.
> think switching to seconds make sense why though? The glib docs say: If you want to have a timer in the "seconds" range and do not care about the exact time of the first call of the timer, use the g_timeout_add_seconds() function and To allow this grouping, the interval to the first timer is rounded and can deviate up to one second from the specified interval. In these one-shot timeout cases, we care *only* about the time to the first call, and since the interval is only 1 or 2 seconds to begin with, a 1-second deviation can eliminate the timeout entirely! Not that it's likely to matter that much either way in practice, but to the extent it does, add_seconds() seems clearly wrong. The bottom line the Ubuntu page seems to encourage a global s/timeout_add/timeout_add_seconds/ but that is just wrong. The purpose of timeout_add_seconds is to deal with *recurring* timeouts. Otherwise, timeout_add_seconds just messes things up, especially for a 1-second-or-less one-shot timeout.
Thanks for the explanation. (Closing Bug) It just looked to me like a easy way to start contributing, guess it's more complicated than it seemed on that page. Places that use 3000 or more should be OK candidates, right? (none in metacity)
yep thanks for the patch of course, I just think the ubuntu page needs to explain a bit more. I think add_seconds should be used when: 1) the timeout is at least 1 second (and can be a round number of seconds) 2) the timeout is recurring i.e. happens over and over in the background If the timeout callback returns "TRUE" that keeps the timeout installed; if "FALSE" that removes the timeout. So if a timeout returns FALSE, usually it is not a recurring timeout.