[gnome-software: 19/25] packagekit: Make a critical section smaller

[Philip Withnall <pwithnall src gnome org>]

commit 86c2b5975bdfbee07595aae06e43de392bb8c69a
Author: Philip Withnall <pwithnall endlessos org>
Date:   Fri Feb 25 15:23:49 2022 +0000

    packagekit: Make a critical section smaller
    
    The critical sections where `prepared_updates_mutex` was locked were too
    long, and were causing noticeable delays when the main thread was
    waiting on acquiring the lock while it was held by another thread.
    
    Rather than updating the `prepared_updates` hash table in place, replace
    it with a new one each time, then each thread can be potentially
    operating (with a strong reference) on independent hash tables from
    successive updates to the cache.
    
    Eventually the locking on `prepared_updates` can go away if all the
    operations in the PackageKit plugin can move into the main thread (if
    that makes sense). Currently, though, it can be accessed from multiple
    worker threads from the vfuncs which haven’t been ported to the new
    threading model yet.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Helps: #1658

 plugins/packagekit/gs-plugin-packagekit.c | 35 ++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 12 deletions(-)
---
diff --git a/plugins/packagekit/gs-plugin-packagekit.c b/plugins/packagekit/gs-plugin-packagekit.c
index 71b4d6553..5b05e44c7 100644
--- a/plugins/packagekit/gs-plugin-packagekit.c
+++ b/plugins/packagekit/gs-plugin-packagekit.c
@@ -1218,13 +1218,13 @@ gs_plugin_systemd_update_cache (GsPluginPackagekit  *self,
 {
        g_autoptr(GError) error_local = NULL;
        g_auto(GStrv) package_ids = NULL;
-       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&self->prepared_updates_mutex);
-
-       /* invalidate */
-       g_hash_table_remove_all (self->prepared_updates);
+       g_autoptr(GHashTable) new_prepared_updates = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        /* get new list of package-ids. This loads a local file, so should be
         * just about fast enough to be sync. */
+       new_prepared_updates = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                                     g_free, NULL);
        package_ids = pk_offline_get_prepared_ids (&error_local);
        if (package_ids == NULL) {
                if (g_error_matches (error_local,
@@ -1241,13 +1241,18 @@ gs_plugin_systemd_update_cache (GsPluginPackagekit  *self,
                return FALSE;
        }
 
+       /* Build the new table, stealing all the elements from @package_ids. */
        for (guint i = 0; package_ids[i] != NULL; i++) {
-               g_hash_table_add (self->prepared_updates, g_steal_pointer (&package_ids[i]));
+               g_hash_table_add (new_prepared_updates, g_steal_pointer (&package_ids[i]));
        }
 
-       /* Already stolen all the elements */
        g_clear_pointer (&package_ids, g_free);
 
+       /* Update the shared state. */
+       locker = g_mutex_locker_new (&self->prepared_updates_mutex);
+       g_clear_pointer (&self->prepared_updates, g_hash_table_unref);
+       self->prepared_updates = g_steal_pointer (&new_prepared_updates);
+
        return TRUE;
 }
 
@@ -1936,7 +1941,7 @@ get_details_cb (GObject      *source_object,
        g_autoptr(GPtrArray) array = NULL;
        g_autoptr(PkResults) results = NULL;
        g_autoptr(GHashTable) details_collection = NULL;
-       g_autoptr(GMutexLocker) locker = NULL;
+       g_autoptr(GHashTable) prepared_updates = NULL;
        g_autoptr(GError) local_error = NULL;
 
        results = pk_client_generic_finish (client, result, &local_error);
@@ -1961,10 +1966,13 @@ get_details_cb (GObject      *source_object,
        details_collection = gs_plugin_packagekit_details_array_to_hash (array);
 
        /* set the update details for the update */
-       locker = g_mutex_locker_new (&self->prepared_updates_mutex);
+       g_mutex_lock (&self->prepared_updates_mutex);
+       prepared_updates = g_hash_table_ref (self->prepared_updates);
+       g_mutex_unlock (&self->prepared_updates_mutex);
+
        for (guint i = 0; i < gs_app_list_length (data->details_list); i++) {
                GsApp *app = gs_app_list_index (data->details_list, i);
-               gs_plugin_packagekit_refine_details_app (GS_PLUGIN (self), details_collection, 
self->prepared_updates, app);
+               gs_plugin_packagekit_refine_details_app (GS_PLUGIN (self), details_collection, 
prepared_updates, app);
        }
 
        refine_task_complete_operation (refine_task);
@@ -2932,16 +2940,19 @@ gs_plugin_url_to_app (GsPlugin *plugin,
 
        if (packages->len >= 1) {
                g_autoptr(GHashTable) details_collection = NULL;
-               g_autoptr(GMutexLocker) locker = NULL;
+               g_autoptr(GHashTable) prepared_updates = NULL;
 
                if (gs_app_get_local_file (app) != NULL)
                        return TRUE;
 
                details_collection = gs_plugin_packagekit_details_array_to_hash (details);
 
+               g_mutex_lock (&self->prepared_updates_mutex);
+               prepared_updates = g_hash_table_ref (self->prepared_updates);
+               g_mutex_unlock (&self->prepared_updates_mutex);
+
                gs_plugin_packagekit_resolve_packages_app (GS_PLUGIN (self), packages, app);
-               locker = g_mutex_locker_new (&self->prepared_updates_mutex);
-               gs_plugin_packagekit_refine_details_app (plugin, details_collection, self->prepared_updates, 
app);
+               gs_plugin_packagekit_refine_details_app (plugin, details_collection, prepared_updates, app);
 
                gs_app_list_add (list, app);
        } else {