From 0810238d22840b227a973b814b6d0ae5c2a00a61 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Sun, 3 Mar 2024 13:50:00 -0800 Subject: [PATCH 1/2] 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 302ca26eb11..4eb20d07688 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 5948c935b4b..09e95b2214c 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 8bd50153e2e..0d98987758a 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') -- GitLab From 1513dd4ef707eaeb0d348296b99692d90c9ccaae Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 4 Mar 2024 11:16:17 -0800 Subject: [PATCH 2/2] wayland: Use timerfd for sub-msec poll() precision Like the change to ClutterFrameClock, this allows poll() timeouts below 1 msec by using a timerfd which will trigger using G_POLL_IN. Part-of: --- src/wayland/meta-wayland.c | 68 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/wayland/meta-wayland.c b/src/wayland/meta-wayland.c index 59c1f5eeec9..7a24cf6b4a0 100644 --- a/src/wayland/meta-wayland.c +++ b/src/wayland/meta-wayland.c @@ -21,11 +21,18 @@ #include "wayland/meta-wayland.h" +#include + #include #include #include #include +#ifdef HAVE_TIMERFD +# include +# include +#endif + #include "clutter/clutter.h" #include "cogl/cogl-egl.h" #include "compositor/meta-surface-actor-wayland.h" @@ -97,6 +104,11 @@ typedef struct MetaWaylandCompositor *compositor; ClutterStageView *stage_view; + +#ifdef HAVE_TIMERFD + int tfd; + struct itimerspec tfd_spec; +#endif } FrameCallbackSource; static void meta_wayland_compositor_update_focus (MetaWaylandCompositor *compositor, @@ -192,6 +204,50 @@ emit_frame_callbacks_for_stage_view (MetaWaylandCompositor *compositor, } #ifdef HAVE_NATIVE_BACKEND + +static gboolean +frame_callback_source_prepare (GSource *base, + int *timeout) +{ + FrameCallbackSource *source = (FrameCallbackSource *)base; + + *timeout = -1; + +#ifdef HAVE_TIMERFD + if (source->tfd > -1) + { + int64_t ready_time = g_source_get_ready_time (base); + 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; + } + + if (memcmp (&tfd_spec, &source->tfd_spec, sizeof tfd_spec) != 0) + { + source->tfd_spec = tfd_spec; + + timerfd_settime (source->tfd, + TFD_TIMER_ABSTIME, + &source->tfd_spec, + NULL); + } + } +#endif + + return FALSE; +} + static gboolean frame_callback_source_dispatch (GSource *source, GSourceFunc callback, @@ -214,9 +270,14 @@ frame_callback_source_finalize (GSource *source) g_signal_handlers_disconnect_by_data (frame_callback_source->stage_view, source); + +#ifdef HAVE_TIMERFD + g_clear_fd (&frame_callback_source->tfd, NULL); +#endif } static GSourceFuncs frame_callback_source_funcs = { + .prepare = frame_callback_source_prepare, .dispatch = frame_callback_source_dispatch, .finalize = frame_callback_source_finalize, }; @@ -260,6 +321,13 @@ frame_callback_source_new (MetaWaylandCompositor *compositor, G_CALLBACK (on_stage_view_destroy), source); +#ifdef HAVE_TIMERFD + frame_callback_source->tfd = timerfd_create (CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC); + + if (frame_callback_source->tfd > -1) + g_source_add_unix_fd (source, frame_callback_source->tfd, G_IO_IN); +#endif + return &frame_callback_source->source; } -- GitLab