[gnome-software/mwleeds/fix-setup-async-parallel] gs-plugin-loader: Don't start all plugins in parallel
- From: Phaedrus Leeds <mwleeds src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software/mwleeds/fix-setup-async-parallel] gs-plugin-loader: Don't start all plugins in parallel
- Date: Tue, 22 Mar 2022 19:47:45 +0000 (UTC)
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]