From c7e2ae30f0f3406f3d841d470be4ec81fb385534 Mon Sep 17 00:00:00 2001 From: Gleb Popov <6yearold@gmail.com> Date: Wed, 24 Jul 2024 15:10:10 +0300 Subject: [PATCH 1/5] Add Meson option that allows selecting GFileMonitor's backend implementation The option defaults to 'auto' which keeps the current selection behavior, but also allows user to override the choice. Also make the chosen backend is reported to the code via CPP defines. --- gio/meson.build | 28 +++++++++++++++++++++++++--- meson.build | 1 + meson.options | 6 ++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/gio/meson.build b/gio/meson.build index d49636d68..447ddd6b2 100644 --- a/gio/meson.build +++ b/gio/meson.build @@ -789,19 +789,41 @@ gioenumtypes_c = custom_target('gioenumtypes_c', gioenumtypes_dep = declare_dependency(sources : [gioenumtypes_h, glib_enumtypes_h, gio_visibility_h]) +file_monitor_backend = get_option('file_monitor_backend') +if file_monitor_backend == 'auto' + if glib_conf.has('HAVE_SYS_INOTIFY_H') and have_func_inotify_init1 + file_monitor_backend = 'inotify' + elif have_func_kqueue and have_func_kevent + file_monitor_backend = 'kqueue' + elif host_system == 'windows' + file_monitor_backend = 'win32' + endif +endif + # inotify -if glib_conf.has('HAVE_SYS_INOTIFY_H') and have_func_inotify_init1 +if file_monitor_backend == 'inotify' + glib_conf.set('FILE_MONITOR_BACKEND_INOTIFY', 1) + subdir('inotify') + internal_deps += [ inotify_lib ] +endif + +# libinotify-kqueue +# uses the same code as inotify, but changes some functions behavior via #ifdef's +if file_monitor_backend == 'libinotify-kqueue' + glib_conf.set('FILE_MONITOR_BACKEND_LIBINOTIFY_KQUEUE', 1) subdir('inotify') internal_deps += [ inotify_lib ] endif # kevent -if have_func_kqueue and have_func_kevent +if file_monitor_backend == 'kqueue' + glib_conf.set('FILE_MONITOR_BACKEND_KQUEUE', 1) subdir('kqueue') internal_deps += [ kqueue_lib ] endif -if host_system == 'windows' +if file_monitor_backend == 'win32' + glib_conf.set('FILE_MONITOR_BACKEND_WIN32', 1) subdir('win32') internal_deps += [ giowin32_lib ] endif diff --git a/meson.build b/meson.build index 2aa4239a9..c9f0404b4 100644 --- a/meson.build +++ b/meson.build @@ -2738,4 +2738,5 @@ summary({ 'libelf' : get_option('libelf'), 'multiarch' : get_option('multiarch'), 'introspection' : enable_gir, + 'file_monitor_backend' : get_option('file_monitor_backend'), }, section: 'Options') diff --git a/meson.options b/meson.options index 93207e0ed..b650eafc5 100644 --- a/meson.options +++ b/meson.options @@ -149,3 +149,9 @@ option('introspection', type: 'feature', value: 'auto', description: 'Enable generating introspection data (requires gobject-introspection)') + +option('file_monitor_backend', + type : 'combo', + choices : ['auto', 'inotify', 'kqueue', 'libinotify-kqueue', 'win32'], + value : 'auto', + description : 'The name of the system API to use as a GFileMonitor backend') From dae3b8bd15fe5f62dc73a3cad66fbb3dba8da084 Mon Sep 17 00:00:00 2001 From: Gleb Popov <6yearold@gmail.com> Date: Wed, 24 Jul 2024 15:29:42 +0300 Subject: [PATCH 2/5] Introduce a special mode of operating for the inotify GFileMonitor backend libinotify-kqueue is a library that implements inotify interface in terms of kqueue/kevent API available on Mac OS and *BSD systems. The original kqueue backend seems to be a predecessor version of the code that is currently present in libinotify-kqueue. Under the hood the library implements a sophisticated filesystem changes detection algorithm that is derived from the glib backend code. Updating the native glib kqueue backend requires substantial work, because code bases have diverged greatly. Another approach is taken, instead. libinotify-kqueue can serve as a drop-in replacement for Linux inotify API, thus allowing to reuse the inotify backend code. The compatibility, however, comes at cost, since the library has to emulate the inotify descriptor via an unix domain socket. This means that delivering an event involves copying the data into the kernel and then pulling it back. The recent libinotify-kqueue release adds a new mode of operation called "direct". In this mode the socket pipe is replaced with another kqueue that is used to deliver events via a kevent(EVFILT_USER) call. Employing the direct mode requires minor changes to the client code compared to using plain inotify API, but in return it allows for reusing libinotify's algorithms without a performance penalty. Luckily, all required changes are consolidated in one file called inotify-kernel.c This puts us in the best of possible worlds. On one hand we share a lot of code with glib inotify backend, which is far more thoroughly tested and widely used. On the other we support a range of non-Linux systems and consolidate the business logic in one library. I plan to do the same trick for QFileSystemWatcher which will give us the same behaviour between Gtk and Qt applications. The glib test suite passes for both old kqueue backend and new libinotify-kqueue one. However, the AppStream FileMonitor tests are failing with the old backend, but pass with the new one, so this is still an observable improvement. Relevant libinotify-kqueue PR: https://github.com/libinotify-kqueue/libinotify-kqueue/pull/19 --- gio/inotify/inotify-kernel.c | 109 ++++++++++++++++++++++++++++++++++- gio/inotify/meson.build | 4 ++ meson.build | 1 + 3 files changed, 111 insertions(+), 3 deletions(-) diff --git a/gio/inotify/inotify-kernel.c b/gio/inotify/inotify-kernel.c index 19eac09c4..90e001ebe 100644 --- a/gio/inotify/inotify-kernel.c +++ b/gio/inotify/inotify-kernel.c @@ -1,6 +1,7 @@ /* Copyright (C) 2005 John McCutchan Copyright © 2015 Canonical Limited + Copyright © 2024 Future Crew LLC SPDX-License-Identifier: LGPL-2.1-or-later @@ -20,6 +21,7 @@ Authors: Ryan Lortie John McCutchan + Gleb Popov */ #include "config.h" @@ -32,6 +34,9 @@ #include #include "inotify-kernel.h" #include +#ifdef HAVE_SYS_UIO_H +#include +#endif #ifdef HAVE_SYS_FILIO_H #include #endif @@ -95,11 +100,11 @@ typedef struct { GSource source; - GQueue queue; + GQueue queue; /* (element-type ik_event_t) */ gpointer fd_tag; gint fd; - GHashTable *unmatched_moves; + GHashTable *unmatched_moves; /* (element-type guint ik_event_t) */ gboolean is_bored; } InotifyKernelSource; @@ -230,6 +235,7 @@ ik_source_dispatch (GSource *source, if (iks->is_bored || g_source_query_unix_fd (source, iks->fd_tag)) { +#if defined(FILE_MONITOR_BACKEND_INOTIFY) gchar stack_buffer[4096]; gsize buffer_len; gchar *buffer; @@ -312,6 +318,78 @@ ik_source_dispatch (GSource *source, if (buffer != stack_buffer) g_free (buffer); +#elif defined(FILE_MONITOR_BACKEND_LIBINOTIFY_KQUEUE) + struct iovec *received[5]; + int num_events = libinotify_direct_readv (iks->fd, received, G_N_ELEMENTS(received), /* no_block=*/ 1); + + if (num_events < 0) + { + int errsv = errno; + g_warning ("Failed to read inotify events: %s", g_strerror (errsv)); + /* fall through and skip the next few blocks */ + } + + for (int i = 0; i < num_events; i++) + { + struct iovec *cur_event = received[i]; + while (cur_event->iov_base) + { + struct inotify_event *kevent = (struct inotify_event *) cur_event->iov_base; + + ik_event_t *event; + + event = ik_event_new (kevent, now); + + if (event->mask & IN_MOVED_TO) + { + ik_event_t *pair; + + pair = g_hash_table_lookup (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie)); + if (pair != NULL) + { + g_assert (!pair->pair); + + g_hash_table_remove (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie)); + event->is_second_in_pair = TRUE; + event->pair = pair; + pair->pair = event; + + cur_event++; + continue; + } + + interesting = TRUE; + } + else if (event->mask & IN_MOVED_FROM) + { + gboolean new; + + new = g_hash_table_insert (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie), event); + if G_UNLIKELY (!new) + g_warning ("inotify: got IN_MOVED_FROM event with already-pending cookie %#x", event->cookie); + + interesting = TRUE; + } + + g_queue_push_tail (&iks->queue, event); + + cur_event++; + } + libinotify_free_iovec (received[i]); + } + + if (num_events == 0) + { + /* We can end up reading nothing if we arrived here due to a + * boredom timer but the stream of events stopped meanwhile. + * + * In that case, we need to switch back to polling the file + * descriptor in the usual way. + */ + g_assert (iks->is_bored); + interesting = TRUE; + } +#endif } while (ik_source_can_dispatch_now (iks, now)) @@ -369,13 +447,30 @@ ik_source_dispatch (GSource *source, return TRUE; } +static void +ik_source_finalize (GSource *source) +{ + InotifyKernelSource *iks; + + iks = (InotifyKernelSource *) source; + +#if defined(FILE_MONITOR_BACKEND_INOTIFY) + close (iks->fd); +#elif defined(FILE_MONITOR_BACKEND_LIBINOTIFY_KQUEUE) + libinotify_direct_close (iks->fd); +#endif + + iks->fd = -1; +} + static InotifyKernelSource * ik_source_new (gboolean (* callback) (ik_event_t *event)) { static GSourceFuncs source_funcs = { NULL, NULL, ik_source_dispatch, - NULL, NULL, NULL + ik_source_finalize, + NULL, NULL }; InotifyKernelSource *iks; GSource *source; @@ -387,23 +482,31 @@ ik_source_new (gboolean (* callback) (ik_event_t *event)) g_source_set_static_name (source, "inotify kernel source"); iks->unmatched_moves = g_hash_table_new (NULL, NULL); +#if defined(FILE_MONITOR_BACKEND_INOTIFY) iks->fd = inotify_init1 (IN_CLOEXEC | IN_NONBLOCK); +#elif defined(FILE_MONITOR_BACKEND_LIBINOTIFY_KQUEUE) + iks->fd = inotify_init1 (IN_CLOEXEC | IN_NONBLOCK | IN_DIRECT); +#endif +#ifdef FILE_MONITOR_BACKEND_INOTIFY if (iks->fd < 0) { should_set_nonblock = TRUE; iks->fd = inotify_init (); } +#endif if (iks->fd >= 0) { GError *error = NULL; +#ifdef FILE_MONITOR_BACKEND_INOTIFY if (should_set_nonblock) { g_unix_set_fd_nonblocking (iks->fd, TRUE, &error); g_assert_no_error (error); } +#endif iks->fd_tag = g_source_add_unix_fd (source, iks->fd, G_IO_IN); } diff --git a/gio/inotify/meson.build b/gio/inotify/meson.build index 4c51c33c9..23e5c886b 100644 --- a/gio/inotify/meson.build +++ b/gio/inotify/meson.build @@ -28,6 +28,9 @@ inotify_sources = [ 'ginotifyfilemonitor.c', ] +# necessary for the libinotify-kqueue backend +libinotify_kqueue_dep = dependency('libinotify', required: file_monitor_backend == 'libinotify-kqueue') + inotify_lib = static_library('inotify', sources : [inotify_sources], include_directories : [configinc, glibinc], @@ -36,6 +39,7 @@ inotify_lib = static_library('inotify', libglib_dep, libgobject_dep, gmodule_inc_dep, + libinotify_kqueue_dep, ], gnu_symbol_visibility : 'hidden', pic : true, diff --git a/meson.build b/meson.build index c9f0404b4..1f1243e56 100644 --- a/meson.build +++ b/meson.build @@ -409,6 +409,7 @@ headers = [ 'strings.h', 'sys/auxv.h', 'sys/event.h', + 'sys/uio.h', 'sys/filio.h', 'sys/inotify.h', 'sys/mkdev.h', From 7460faf8612b385872718b969e258db301f7ce33 Mon Sep 17 00:00:00 2001 From: Gleb Popov <6yearold@gmail.com> Date: Wed, 24 Jul 2024 15:57:46 +0300 Subject: [PATCH 3/5] giomodule: Adapt to the GFileMintor backend selection changes Instead of relying on some headers/functions presence, the buildsystem now tells us what backend we're using. --- gio/giomodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gio/giomodule.c b/gio/giomodule.c index aa6272f3c..e19fc9065 100644 --- a/gio/giomodule.c +++ b/gio/giomodule.c @@ -1340,10 +1340,10 @@ _g_io_modules_ensure_loaded (void) g_type_ensure (g_memory_settings_backend_get_type ()); g_type_ensure (g_keyfile_settings_backend_get_type ()); g_type_ensure (g_power_profile_monitor_dbus_get_type ()); -#if defined(HAVE_INOTIFY_INIT1) +#if defined(FILE_MONITOR_BACKEND_INOTIFY) || defined(FILE_MONITOR_BACKEND_LIBINOTIFY_KQUEUE) g_type_ensure (g_inotify_file_monitor_get_type ()); #endif -#if defined(HAVE_KQUEUE) +#if defined(FILE_MONITOR_BACKEND_KQUEUE) g_type_ensure (g_kqueue_file_monitor_get_type ()); #endif #ifdef G_OS_WIN32 From 697118dfd5c06c10d8f04b73736e98167eccf378 Mon Sep 17 00:00:00 2001 From: Gleb Popov <6yearold@gmail.com> Date: Wed, 24 Jul 2024 16:02:30 +0300 Subject: [PATCH 4/5] Adapt testfilemonitor.c for libinotify-kqueue backend The backend mimics inotify but is still subject to kevent idionsyncracies. --- gio/tests/testfilemonitor.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gio/tests/testfilemonitor.c b/gio/tests/testfilemonitor.c index 52a530244..018e49f50 100644 --- a/gio/tests/testfilemonitor.c +++ b/gio/tests/testfilemonitor.c @@ -116,11 +116,15 @@ static const gchar DONT_CARE[] = ""; static Environment get_environment (GFileMonitor *monitor) { - if (g_str_equal (G_OBJECT_TYPE_NAME (monitor), "GInotifyFileMonitor")) +#if defined(FILE_MONITOR_BACKEND_INOTIFY) return INOTIFY; - if (g_str_equal (G_OBJECT_TYPE_NAME (monitor), "GKqueueFileMonitor")) +#elif defined(FILE_MONITOR_BACKEND_KQUEUE) return KQUEUE; +#elif defined(FILE_MONITOR_BACKEND_LIBINOTIFY_KQUEUE) + return INOTIFY | KQUEUE; +#else return NONE; +#endif } static void From 30dfc99c941d4a8081216b4cfb0162c571a1c8e8 Mon Sep 17 00:00:00 2001 From: Gleb Popov <6yearold@gmail.com> Date: Thu, 19 Sep 2024 10:22:39 +0300 Subject: [PATCH 5/5] inotify: Optimize consecutive g_hash_table_{lookup,remove} calls --- gio/inotify/inotify-kernel.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gio/inotify/inotify-kernel.c b/gio/inotify/inotify-kernel.c index 90e001ebe..fc78641db 100644 --- a/gio/inotify/inotify-kernel.c +++ b/gio/inotify/inotify-kernel.c @@ -275,12 +275,10 @@ ik_source_dispatch (GSource *source, { ik_event_t *pair; - pair = g_hash_table_lookup (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie)); - if (pair != NULL) + if (g_hash_table_steal_extended (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie), NULL, (gpointer*)&pair)) { g_assert (!pair->pair); - g_hash_table_remove (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie)); event->is_second_in_pair = TRUE; event->pair = pair; pair->pair = event; @@ -344,12 +342,10 @@ ik_source_dispatch (GSource *source, { ik_event_t *pair; - pair = g_hash_table_lookup (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie)); - if (pair != NULL) + if (g_hash_table_steal_extended (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie), NULL, (gpointer*)&pair)) { g_assert (!pair->pair); - g_hash_table_remove (iks->unmatched_moves, GUINT_TO_POINTER (event->cookie)); event->is_second_in_pair = TRUE; event->pair = pair; pair->pair = event;