From 0810238d22840b227a973b814b6d0ae5c2a00a61 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Sun, 3 Mar 2024 13:50:00 -0800 Subject: [PATCH] clutter/frame-clock: Use timerfd for clock timing Currently, ClutterFrameClock uses g_source_set_ready_time() to determine the usec timing of the next frame. That translates into a poll() with a millisecond timeout if no trigger occurs to break the poll() out early. To avoid spinning the CPU, GLib always rounds *up* to the next millisecond value unless a timeout of 0 was provided by a GSource. This means that timeouts for the ClutterFrameClock can easily skew beyond their expected time as the precision is too coarse. This applies the same concept as GNOME/glib!3949 but just for the ClutterFrameClock. That may be more ideal than adding a timerfd for every GMainContext, but we'll see if that lands upstream. I wanted to provide this here because it could easily be cherry-picked in the mean time if this is found to be useful. From a timer stability perspective, this improves things from erratically jumping between 100s and 1000s off of the expected awake time to single or low double digits. Part-of: --- clutter/clutter/clutter-frame-clock.c | 93 ++++++++++++++++++++++++++- config.h.meson | 3 + meson.build | 14 ++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/clutter/clutter/clutter-frame-clock.c b/clutter/clutter/clutter-frame-clock.c index 302ca26eb..4eb20d076 100644 --- a/clutter/clutter/clutter-frame-clock.c +++ b/clutter/clutter/clutter-frame-clock.c @@ -19,6 +19,13 @@ #include "clutter/clutter-frame-clock.h" +#include + +#ifdef HAVE_TIMERFD +#include +#include +#endif + #include "clutter/clutter-debug.h" #include "clutter/clutter-frame-private.h" #include "clutter/clutter-main.h" @@ -50,6 +57,11 @@ typedef struct _ClutterClockSource GSource source; ClutterFrameClock *frame_clock; + +#ifdef HAVE_TIMERFD + int tfd; + struct itimerspec tfd_spec; +#endif } ClutterClockSource; typedef enum _ClutterFrameClockState @@ -1051,11 +1063,81 @@ clutter_frame_clock_get_max_render_time_debug_info (ClutterFrameClock *frame_clo return string; } +static gboolean +frame_clock_source_prepare (GSource *source, + int *timeout) +{ + G_GNUC_UNUSED ClutterClockSource *clock_source = (ClutterClockSource *)source; + + *timeout = -1; + +#ifdef HAVE_TIMERFD + /* The cycle for GMainContext is: + * + * - prepare(): where we update our timerfd deadline + * - poll(): internal to GMainContext/GPollFunc + * - check(): where GLib will check POLLIN and make ready + * - dispatch(): where we actually process the pending work + * + * If we have a ready_time >= 0 then we need to set our deadline + * in nanoseconds for the timerfd. The timerfd will receive POLLIN + * after that point and poll() will return. + * + * If we have a ready_time of -1, then we need to disable our + * timerfd by setting tv_sec and tv_nsec to 0. + * + * In both cases, the POLLIN bit will be reset. + */ + if (clock_source->tfd > -1) + { + int64_t ready_time = g_source_get_ready_time (source); + struct itimerspec tfd_spec; + + tfd_spec.it_interval.tv_sec = 0; + tfd_spec.it_interval.tv_nsec = 0; + + if (ready_time > -1) + { + tfd_spec.it_value.tv_sec = ready_time / G_USEC_PER_SEC; + tfd_spec.it_value.tv_nsec = (ready_time % G_USEC_PER_SEC) * 1000L; + } + else + { + tfd_spec.it_value.tv_sec = 0; + tfd_spec.it_value.tv_nsec = 0; + } + + /* Avoid extraneous calls timerfd_settime() */ + if (memcmp (&tfd_spec, &clock_source->tfd_spec, sizeof tfd_spec) != 0) + { + clock_source->tfd_spec = tfd_spec; + + timerfd_settime (clock_source->tfd, + TFD_TIMER_ABSTIME, + &clock_source->tfd_spec, + NULL); + } + } +#endif + + return FALSE; +} + +static void +frame_clock_source_finalize (GSource *source) +{ +#ifdef HAVE_TIMERFD + ClutterClockSource *clock_source = (ClutterClockSource *)source; + + g_clear_fd (&clock_source->tfd, NULL); +#endif +} + static GSourceFuncs frame_clock_source_funcs = { - NULL, + frame_clock_source_prepare, NULL, frame_clock_source_dispatch, - NULL + frame_clock_source_finalize, }; static void @@ -1068,6 +1150,13 @@ init_frame_clock_source (ClutterFrameClock *frame_clock) source = g_source_new (&frame_clock_source_funcs, sizeof (ClutterClockSource)); clock_source = (ClutterClockSource *) source; +#ifdef HAVE_TIMERFD + clock_source->tfd = timerfd_create (CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC); + + if (clock_source->tfd > -1) + g_source_add_unix_fd (source, clock_source->tfd, G_IO_IN); +#endif + name = g_strdup_printf ("[mutter] Clutter frame clock (%p)", frame_clock); g_source_set_name (source, name); g_source_set_priority (source, CLUTTER_PRIORITY_REDRAW); diff --git a/config.h.meson b/config.h.meson index 5948c935b..09e95b221 100644 --- a/config.h.meson +++ b/config.h.meson @@ -129,3 +129,6 @@ /* Supports PangoFt2 */ #mesondefine HAVE_PANGO_FT2 + +/* Supports timerfd_create/timerfd_settime */ +#mesondefine HAVE_TIMERFD diff --git a/meson.build b/meson.build index 8bd50153e..0d9898775 100644 --- a/meson.build +++ b/meson.build @@ -331,6 +331,19 @@ if have_introspection } endif +# Check for timerfd_create(2) +have_timerfd = cc.links(''' +#include +#include +#include +int main (int argc, char ** argv) { + struct itimerspec ts = {{0}}; + int fd = timerfd_create (CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC); + timerfd_settime (fd, TFD_TIMER_ABSTIME, &ts, NULL); + return 0; +} +''', name : 'timerfd_create(2) system call') + have_documentation = get_option('docs') have_tests = get_option('tests') have_core_tests = false @@ -551,6 +564,7 @@ cdata.set('HAVE_INTROSPECTION', have_introspection) cdata.set('HAVE_PROFILER', have_profiler) cdata.set('HAVE_LIBDISPLAY_INFO', have_libdisplay_info) cdata.set('HAVE_PANGO_FT2', have_pango_ft2) +cdata.set('HAVE_TIMERFD', have_timerfd) if have_x11_client xkb_base = xkeyboard_config_dep.get_variable('xkb_base')