[gnome-software/mwleeds/fix-setup-async-parallel] gs-plugin-loader: Don't start all plugins in parallel




commit 9d5a555f142a98fb9b3d913ed52f62e0d23fe4b6
Author: Phaedrus Leeds <mwleeds protonmail com>
Date:   Tue Mar 22 12:07:06 2022 -0700

    gs-plugin-loader: Don't start all plugins in parallel
    
    Plugins use rules like this to declare their ordering requirements:
    
    gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_RUN_AFTER, "appstream");
    
    And in gs_plugin_loader_setup_async() those rules are used to set an
    order number on each plugin and the list of plugins is then sorted by
    order number. In GNOME Software 41, the plugins were then set up
    synchronously in series, so plugins later in the list were not set up
    until after plugins earlier in the list had finished setting up. But in
    g-s 42 we moved to doing asynchronous setup of each plugin, which means
    that plugins can no longer depend on their declared
    GS_PLUGIN_RULE_RUN_AFTER or GS_PLUGIN_RULE_RUN_BEFORE dependencies being
    fully set up after or before them, respectively.
    
    This is a bug, and it is making the epiphany plugin harder to reason
    about, so fix it by setting up batches of plugins based on the order
    numbers: plugins within a batch are set up asynchronously in parallel,
    and the next batch is not started until all plugins in the previous
    batch have finished.

 lib/gs-plugin-loader.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 5 deletions(-)
---
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 3bc93b8cb..a94ffcbaa 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -2142,6 +2142,7 @@ gs_plugin_loader_find_plugins (const gchar *path, GError **error)
 
 typedef struct {
        guint n_pending;
+       guint current_batch;
 #ifdef HAVE_SYSPROF
        gint64 setup_begin_time_nsec;
        gint64 plugins_begin_time_nsec;
@@ -2429,6 +2430,7 @@ gs_plugin_loader_setup_async (GsPluginLoader      *plugin_loader,
        /* run setup */
        setup_data = setup_data_owned = g_new0 (SetupData, 1);
        setup_data->n_pending = 1;  /* incremented until all operations have been started */
+       setup_data->current_batch = G_MAXUINT;
 #ifdef HAVE_SYSPROF
        setup_data->setup_begin_time_nsec = begin_time_nsec;
        setup_data->plugins_begin_time_nsec = SYSPROF_CAPTURE_CURRENT_TIME;
@@ -2436,17 +2438,28 @@ gs_plugin_loader_setup_async (GsPluginLoader      *plugin_loader,
 
        g_task_set_task_data (task, g_steal_pointer (&setup_data_owned), (GDestroyNotify) setup_data_free);
 
+       /* Set up the first batch of plugins as determined by order number set above */
        for (i = 0; i < plugin_loader->plugins->len; i++) {
+               guint plugin_order;
                plugin = GS_PLUGIN (plugin_loader->plugins->pdata[i]);
 
                if (!gs_plugin_get_enabled (plugin))
                        continue;
 
-               if (GS_PLUGIN_GET_CLASS (plugin)->setup_async != NULL) {
-                       setup_data->n_pending++;
-                       GS_PLUGIN_GET_CLASS (plugin)->setup_async (plugin, cancellable,
-                                                                  plugin_setup_cb, g_object_ref (task));
-               }
+               if (GS_PLUGIN_GET_CLASS (plugin)->setup_async == NULL)
+                       continue;
+
+               plugin_order = gs_plugin_get_order (plugin);
+               if (setup_data->current_batch == G_MAXUINT)
+                       setup_data->current_batch = plugin_order;
+               else if (setup_data->current_batch != plugin_order)
+                       break;
+
+               setup_data->n_pending++;
+               g_debug ("Setting up plugin %s in batch %u",
+                        gs_plugin_get_name (plugin), plugin_order);
+               GS_PLUGIN_GET_CLASS (plugin)->setup_async (plugin, cancellable,
+                                                          plugin_setup_cb, g_object_ref (task));
        }
 
        finish_setup_op (task);
@@ -2497,6 +2510,45 @@ finish_setup_op (GTask *task)
        GCancellable *cancellable = g_task_get_cancellable (task);
        g_autoptr(GsAppList) install_queue = NULL;
        g_autoptr(GError) local_error = NULL;
+       GsPlugin *plugin;
+       guint i;
+       guint prev_batch = data->current_batch;
+
+       g_assert (data->n_pending > 0);
+       data->n_pending--;
+
+       if (data->n_pending > 0)
+               return;
+
+       data->n_pending = 1; /* increment while starting the batch */
+       data->current_batch = G_MAXUINT;
+
+       /* Check if there's another batch of plugins to set up */
+       for (i = 0; i < plugin_loader->plugins->len; i++) {
+               guint plugin_order;
+               plugin = GS_PLUGIN (plugin_loader->plugins->pdata[i]);
+
+               if (!gs_plugin_get_enabled (plugin))
+                       continue;
+
+               if (GS_PLUGIN_GET_CLASS (plugin)->setup_async == NULL)
+                       continue;
+
+               plugin_order = gs_plugin_get_order (plugin);
+               if (plugin_order <= prev_batch)
+                       continue;
+
+               if (data->current_batch == G_MAXUINT)
+                       data->current_batch = plugin_order;
+               else if (plugin_order > data->current_batch)
+                       break;
+
+               data->n_pending++;
+               g_debug ("Setting up plugin %s in batch %u",
+                        gs_plugin_get_name (plugin), plugin_order);
+               GS_PLUGIN_GET_CLASS (plugin)->setup_async (plugin, cancellable,
+                                                          plugin_setup_cb, g_object_ref (task));
+       }
 
        g_assert (data->n_pending > 0);
        data->n_pending--;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]