From 4e42df8318b98b3fd27c1a9f392b4dd150a011fe Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 Apr 2020 13:59:44 +0100 Subject: [PATCH 1/4] gs-category-page: Block signal emission while reloading Rather than disconnecting and reconnecting, which is more work and could lead to signal handler leaks if, for example, `gs_category_page_get_apps_cb()` returns early due to error. Signed-off-by: Philip Withnall --- src/gs-category-page.c | 44 ++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/gs-category-page.c b/src/gs-category-page.c index 10467436c..dec34328a 100644 --- a/src/gs-category-page.c +++ b/src/gs-category-page.c @@ -157,14 +157,8 @@ gs_category_page_get_apps_cb (GObject *source_object, gtk_widget_set_can_focus (gtk_widget_get_parent (tile), FALSE); } - self->sort_rating_handler_id = g_signal_connect (self->sort_rating_button, - "clicked", - G_CALLBACK (sort_button_clicked), - self); - self->sort_name_handler_id = g_signal_connect (self->sort_name_button, - "clicked", - G_CALLBACK (sort_button_clicked), - self); + g_signal_handler_unblock (self->sort_rating_button, self->sort_rating_handler_id); + g_signal_handler_unblock (self->sort_name_button, self->sort_name_handler_id); } static gboolean @@ -321,17 +315,8 @@ gs_category_page_reload (GsPage *page) gtk_widget_set_visible (self->subcats_sort_button, TRUE); } - if (self->sort_rating_handler_id > 0) { - g_signal_handler_disconnect (self->sort_rating_button, - self->sort_rating_handler_id); - self->sort_rating_handler_id = 0; - } - - if (self->sort_name_handler_id > 0) { - g_signal_handler_disconnect (self->sort_name_button, - self->sort_name_handler_id); - self->sort_name_handler_id = 0; - } + g_signal_handler_block (self->sort_rating_button, self->sort_rating_handler_id); + g_signal_handler_block (self->sort_name_button, self->sort_name_handler_id); gs_container_remove_all (GTK_CONTAINER (self->category_detail_box)); @@ -516,6 +501,18 @@ gs_category_page_dispose (GObject *object) g_cancellable_cancel (self->cancellable); g_clear_object (&self->cancellable); + if (self->sort_rating_handler_id > 0) { + g_signal_handler_disconnect (self->sort_rating_button, + self->sort_rating_handler_id); + self->sort_rating_handler_id = 0; + } + + if (self->sort_name_handler_id > 0) { + g_signal_handler_disconnect (self->sort_name_button, + self->sort_name_handler_id); + self->sort_name_handler_id = 0; + } + g_clear_object (&self->builder); g_clear_object (&self->category); g_clear_object (&self->subcategory); @@ -543,6 +540,15 @@ gs_category_page_setup (GsPage *page, gs_category_page_sort_flow_box_sort_func, self, NULL); + self->sort_rating_handler_id = g_signal_connect (self->sort_rating_button, + "clicked", + G_CALLBACK (sort_button_clicked), + self); + self->sort_name_handler_id = g_signal_connect (self->sort_name_button, + "clicked", + G_CALLBACK (sort_button_clicked), + self); + adj = gtk_scrolled_window_get_vadjustment (GTK_SCROLLED_WINDOW (self->scrolledwindow_category)); gtk_container_set_focus_vadjustment (GTK_CONTAINER (self->category_detail_box), adj); return TRUE; -- GitLab From cfc3766ab770fe98be47128e6d31936310eb6854 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 Apr 2020 10:32:26 +0100 Subject: [PATCH 2/4] gs-plugin-loader: Iterate over hash tables directly `g_hash_table_get_{keys,values}()` are quite inefficient: they have to iterate over the hash table and copy the keys/values into a newly allocated linked list, return that, and then the calling code has to iterate over that again. Instead, use `GHashTableIter`, which does zero allocations and iterates directly in the calling code. This introduces no functional changes. Signed-off-by: Philip Withnall --- lib/gs-plugin-loader.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c index f0e124cbc..59c3d3e65 100644 --- a/lib/gs-plugin-loader.c +++ b/lib/gs-plugin-loader.c @@ -1823,16 +1823,17 @@ gboolean gs_plugin_loader_get_allow_updates (GsPluginLoader *plugin_loader) { GsPluginLoaderPrivate *priv = gs_plugin_loader_get_instance_private (plugin_loader); - g_autoptr(GList) list = NULL; + GHashTableIter iter; + gpointer value; /* nothing */ if (g_hash_table_size (priv->disallow_updates) == 0) return TRUE; /* list */ - list = g_hash_table_get_values (priv->disallow_updates); - for (GList *l = list; l != NULL; l = l->next) { - const gchar *reason = l->data; + g_hash_table_iter_init (&iter, priv->disallow_updates); + while (g_hash_table_iter_next (&iter, NULL, &value)) { + const gchar *reason = value; g_debug ("managed updates inhibited by %s", reason); } return FALSE; @@ -1880,16 +1881,17 @@ gs_plugin_loader_get_events (GsPluginLoader *plugin_loader) { GsPluginLoaderPrivate *priv = gs_plugin_loader_get_instance_private (plugin_loader); GPtrArray *events = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref); - g_autoptr(GList) keys = NULL; g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->events_by_id_mutex); + GHashTableIter iter; + gpointer key, value; /* just add everything */ - keys = g_hash_table_get_keys (priv->events_by_id); - for (GList *l = keys; l != NULL; l = l->next) { - const gchar *key = l->data; - GsPluginEvent *event = g_hash_table_lookup (priv->events_by_id, key); + g_hash_table_iter_init (&iter, priv->events_by_id); + while (g_hash_table_iter_next (&iter, &key, &value)) { + const gchar *id = key; + GsPluginEvent *event = value; if (event == NULL) { - g_warning ("failed to get event for '%s'", key); + g_warning ("failed to get event for '%s'", id); continue; } g_ptr_array_add (events, g_object_ref (event)); @@ -1910,16 +1912,17 @@ GsPluginEvent * gs_plugin_loader_get_event_default (GsPluginLoader *plugin_loader) { GsPluginLoaderPrivate *priv = gs_plugin_loader_get_instance_private (plugin_loader); - g_autoptr(GList) keys = NULL; g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->events_by_id_mutex); + GHashTableIter iter; + gpointer key, value; /* just add everything */ - keys = g_hash_table_get_keys (priv->events_by_id); - for (GList *l = keys; l != NULL; l = l->next) { - const gchar *key = l->data; - GsPluginEvent *event = g_hash_table_lookup (priv->events_by_id, key); + g_hash_table_iter_init (&iter, priv->events_by_id); + while (g_hash_table_iter_next (&iter, &key, &value)) { + const gchar *id = key; + GsPluginEvent *event = value; if (event == NULL) { - g_warning ("failed to get event for '%s'", key); + g_warning ("failed to get event for '%s'", id); continue; } if (!gs_plugin_event_has_flag (event, GS_PLUGIN_EVENT_FLAG_INVALID)) -- GitLab From 1e096d1fa82173bf19fa929d670873ab3037f117 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 Apr 2020 10:41:39 +0100 Subject: [PATCH 3/4] gs-plugin-loader: Rework some hash table code to use fewer calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This won’t affect performance much, but does make the code a little cleaner. There’s no need to query whether an element exists in a hash table before removing it or updating it: instead, check the return value from the remove/update function. Signed-off-by: Philip Withnall --- lib/gs-plugin-loader.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c index 59c3d3e65..c8136f1cd 100644 --- a/lib/gs-plugin-loader.c +++ b/lib/gs-plugin-loader.c @@ -1962,30 +1962,30 @@ gs_plugin_loader_allow_updates_cb (GsPlugin *plugin, GsPluginLoader *plugin_loader) { GsPluginLoaderPrivate *priv = gs_plugin_loader_get_instance_private (plugin_loader); - gpointer exists; + gboolean changed = FALSE; /* plugin now allowing gnome-software to show updates panel */ - exists = g_hash_table_lookup (priv->disallow_updates, plugin); if (allow_updates) { - if (exists == NULL) - return; - g_debug ("plugin %s no longer inhibited managed updates", - gs_plugin_get_name (plugin)); - g_hash_table_remove (priv->disallow_updates, plugin); + if (g_hash_table_remove (priv->disallow_updates, plugin)) { + g_debug ("plugin %s no longer inhibited managed updates", + gs_plugin_get_name (plugin)); + changed = TRUE; + } /* plugin preventing the updates panel from being shown */ } else { - if (exists != NULL) - return; - g_debug ("plugin %s inhibited managed updates", - gs_plugin_get_name (plugin)); - g_hash_table_insert (priv->disallow_updates, - (gpointer) plugin, - (gpointer) gs_plugin_get_name (plugin)); + if (g_hash_table_replace (priv->disallow_updates, + (gpointer) plugin, + (gpointer) gs_plugin_get_name (plugin))) { + g_debug ("plugin %s inhibited managed updates", + gs_plugin_get_name (plugin)); + changed = TRUE; + } } - /* something possibly changed, so notify display layer */ - g_object_notify (G_OBJECT (plugin_loader), "allow-updates"); + /* notify display layer */ + if (changed) + g_object_notify (G_OBJECT (plugin_loader), "allow-updates"); } static void -- GitLab From ca4a24dfdf8c8ff53904cbfa8767de97f29ceb7b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 Apr 2020 10:43:49 +0100 Subject: [PATCH 4/4] flatpak: Drop a debug message It was getting emitted a lot (possibly on every refine operation), slowing things down a little, and clogging up the `--verbose` output. Signed-off-by: Philip Withnall --- plugins/flatpak/gs-flatpak.c | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index 725993275..20afb5aca 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -2389,7 +2389,6 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, for (guint i = 0; i < components->len; i++) { XbNode *component = g_ptr_array_index (components, i); g_autoptr(GsApp) new = NULL; - g_debug ("found component for wildcard %s", id); new = gs_appstream_create_app (self->plugin, self->silo, component, error); if (new == NULL) return FALSE; -- GitLab