From d8563228449501002e95ac372856ed8295ecfcd4 Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Mon, 14 Sep 2020 12:12:15 +0200 Subject: [PATCH] Revert an optimization that broke packagekit updates --- ...t-Avoid-600000-allocations-when-comp.patch | 274 ++++++++++++++++++ gnome-software.spec | 10 +- 2 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 0001-Revert-packagekit-Avoid-600000-allocations-when-comp.patch diff --git a/0001-Revert-packagekit-Avoid-600000-allocations-when-comp.patch b/0001-Revert-packagekit-Avoid-600000-allocations-when-comp.patch new file mode 100644 index 0000000..0d293bb --- /dev/null +++ b/0001-Revert-packagekit-Avoid-600000-allocations-when-comp.patch @@ -0,0 +1,274 @@ +From b3a50ee2d6b93980d1808599ba003e9afc4feae5 Mon Sep 17 00:00:00 2001 +From: Kalev Lember +Date: Mon, 14 Sep 2020 12:07:30 +0200 +Subject: [PATCH] Revert "packagekit: Avoid 600000 allocations when comparing + package IDs" + +This broke packagekit updates. + +https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1061 +https://bodhi.fedoraproject.org/updates/FEDORA-2020-7f57486c95#comment-1621958 + +This reverts commit 955570e4a5d737a9a4f85860fd7e483158e130c4. +--- + .../packagekit/gs-plugin-packagekit-refine.c | 12 +- + .../gs-plugin-packagekit-url-to-app.c | 6 +- + plugins/packagekit/packagekit-common.c | 149 ++++++------------ + plugins/packagekit/packagekit-common.h | 3 +- + 4 files changed, 54 insertions(+), 116 deletions(-) + +diff --git a/plugins/packagekit/gs-plugin-packagekit-refine.c b/plugins/packagekit/gs-plugin-packagekit-refine.c +index 68f7eb64..813390b1 100644 +--- a/plugins/packagekit/gs-plugin-packagekit-refine.c ++++ b/plugins/packagekit/gs-plugin-packagekit-refine.c +@@ -345,7 +345,6 @@ gs_plugin_packagekit_refine_details2 (GsPlugin *plugin, + g_autoptr(GPtrArray) array = NULL; + g_autoptr(GPtrArray) package_ids = NULL; + g_autoptr(PkResults) results = NULL; +- g_autoptr(GHashTable) details_collection = NULL; + + package_ids = g_ptr_array_new_with_free_func (g_free); + for (i = 0; i < gs_app_list_length (list); i++) { +@@ -375,19 +374,12 @@ gs_plugin_packagekit_refine_details2 (GsPlugin *plugin, + return FALSE; + } + +- /* get the results and copy them into a hash table for fast lookups: +- * there are typically 400 to 700 elements in @array, and 100 to 200 +- * elements in @list, each with 1 or 2 source IDs to look up (but +- * sometimes 200) */ +- array = pk_results_get_details_array (results); +- details_collection = gs_plugin_packagekit_details_array_to_hash (array); +- + /* set the update details for the update */ ++ array = pk_results_get_details_array (results); + for (i = 0; i < gs_app_list_length (list); i++) { + app = gs_app_list_index (list, i); +- gs_plugin_packagekit_refine_details_app (plugin, details_collection, app); ++ gs_plugin_packagekit_refine_details_app (plugin, array, app); + } +- + return TRUE; + } + +diff --git a/plugins/packagekit/gs-plugin-packagekit-url-to-app.c b/plugins/packagekit/gs-plugin-packagekit-url-to-app.c +index 04189204..7f566c72 100644 +--- a/plugins/packagekit/gs-plugin-packagekit-url-to-app.c ++++ b/plugins/packagekit/gs-plugin-packagekit-url-to-app.c +@@ -106,15 +106,11 @@ gs_plugin_url_to_app (GsPlugin *plugin, + details = pk_results_get_details_array (results); + + if (packages->len >= 1) { +- g_autoptr(GHashTable) details_collection = NULL; +- + if (gs_app_get_local_file (app) != NULL) + return TRUE; + +- details_collection = gs_plugin_packagekit_details_array_to_hash (details); +- + gs_plugin_packagekit_resolve_packages_app (plugin, packages, app); +- gs_plugin_packagekit_refine_details_app (plugin, details_collection, app); ++ gs_plugin_packagekit_refine_details_app (plugin, details, app); + + gs_app_list_add (list, app); + } else { +diff --git a/plugins/packagekit/packagekit-common.c b/plugins/packagekit/packagekit-common.c +index 495960dd..9367f5bf 100644 +--- a/plugins/packagekit/packagekit-common.c ++++ b/plugins/packagekit/packagekit-common.c +@@ -388,127 +388,78 @@ gs_plugin_packagekit_set_metadata_from_package (GsPlugin *plugin, + pk_package_get_summary (package)); + } + +-/* Hash functions which compare PkPackageIds on NAME, VERSION and ARCH, but not DATA. +- * This is because some backends do not append the origin. ++/* ++ * gs_pk_compare_ids: + * +- * Borrowing some implementation details from pk-package-id.c, a package +- * ID is a semicolon-separated list of NAME;[VERSION];[ARCH];[DATA], +- * so a comparison which ignores DATA is just a strncmp() up to and +- * including the final semicolon. +- * +- * Doing it this way means zero allocations, which allows the hash and +- * equality functions to be fast. This is important when dealing with +- * large refine() package lists. +- * +- * The hash and equality functions assume that the IDs they are passed are +- * valid. */ +-static guint +-package_id_hash (gconstpointer key) +-{ +- const gchar *package_id = key; +- gchar *no_data; +- gsize i, last_semicolon = 0; +- +- /* find the last semicolon, which starts the DATA section */ +- for (i = 0; package_id[i] != '\0'; i++) { +- if (package_id[i] == ';') +- last_semicolon = i; +- } +- +- /* exit early if the DATA section was empty */ +- if (last_semicolon + 1 == i) +- return g_str_hash (package_id); +- +- /* extract up to (and including) the last semicolon into a local string */ +- no_data = g_alloca (last_semicolon + 2); +- memcpy (no_data, package_id, last_semicolon + 1); +- no_data[last_semicolon + 1] = '\0'; +- +- return g_str_hash (no_data); +-} +- ++ * Do not compare the repo. Some backends do not append the origin. ++ */ + static gboolean +-package_id_equal (gconstpointer a, +- gconstpointer b) ++gs_pk_compare_ids (const gchar *package_id1, const gchar *package_id2) + { +- const gchar *package_id_a = a; +- const gchar *package_id_b = b; +- gsize n_semicolons = 0; +- +- /* compare up to and including the last semicolon */ +- for (gsize i = 0; package_id_a[i] != '\0' && package_id_b[i] != '\0'; i++) { +- if (package_id_a[i] != package_id_b[i]) +- return FALSE; +- if (package_id_a[i] == ';') +- n_semicolons++; +- if (n_semicolons == 4) +- return TRUE; +- } ++ gboolean ret; ++ g_auto(GStrv) split1 = NULL; ++ g_auto(GStrv) split2 = NULL; + +- return FALSE; ++ split1 = pk_package_id_split (package_id1); ++ if (split1 == NULL) ++ return FALSE; ++ split2 = pk_package_id_split (package_id2); ++ if (split2 == NULL) ++ return FALSE; ++ ret = (g_strcmp0 (split1[PK_PACKAGE_ID_NAME], ++ split2[PK_PACKAGE_ID_NAME]) == 0 && ++ g_strcmp0 (split1[PK_PACKAGE_ID_VERSION], ++ split2[PK_PACKAGE_ID_VERSION]) == 0 && ++ g_strcmp0 (split1[PK_PACKAGE_ID_ARCH], ++ split2[PK_PACKAGE_ID_ARCH]) == 0); ++ return ret; + } + +-GHashTable * +-gs_plugin_packagekit_details_array_to_hash (GPtrArray *array) +-{ +- g_autoptr(GHashTable) details_collection = NULL; +- +- details_collection = g_hash_table_new_full (package_id_hash, package_id_equal, +- NULL, NULL); +- +- for (gsize i = 0; i < array->len; i++) { +- PkDetails *details = g_ptr_array_index (array, i); +- g_hash_table_insert (details_collection, +- pk_details_get_package_id (details), +- details); +- } +- +- return g_steal_pointer (&details_collection); +-} + + void + gs_plugin_packagekit_refine_details_app (GsPlugin *plugin, +- GHashTable *details_collection, ++ GPtrArray *array, + GsApp *app) + { + GPtrArray *source_ids; + PkDetails *details; + const gchar *package_id; ++ guint i; + guint j; + guint64 size = 0; + +- /* @source_ids can have as many as 200 elements (google-noto); typically +- * it has 1 or 2 +- * +- * @details_collection is typically a large list of apps in the +- * repository, on the order of 400 or 700 apps */ + source_ids = gs_app_get_source_ids (app); + for (j = 0; j < source_ids->len; j++) { + package_id = g_ptr_array_index (source_ids, j); +- details = g_hash_table_lookup (details_collection, package_id); +- if (details == NULL) +- continue; +- +- if (gs_app_get_license (app) == NULL) { +- g_autofree gchar *license_spdx = NULL; +- license_spdx = as_utils_license_to_spdx (pk_details_get_license (details)); +- if (license_spdx != NULL) { +- gs_app_set_license (app, +- GS_APP_QUALITY_LOWEST, +- license_spdx); ++ for (i = 0; i < array->len; i++) { ++ /* right package? */ ++ details = g_ptr_array_index (array, i); ++ if (!gs_pk_compare_ids (package_id, ++ pk_details_get_package_id (details))) { ++ continue; + } ++ if (gs_app_get_license (app) == NULL) { ++ g_autofree gchar *license_spdx = NULL; ++ license_spdx = as_utils_license_to_spdx (pk_details_get_license (details)); ++ if (license_spdx != NULL) { ++ gs_app_set_license (app, ++ GS_APP_QUALITY_LOWEST, ++ license_spdx); ++ } ++ } ++ if (gs_app_get_url (app, AS_URL_KIND_HOMEPAGE) == NULL) { ++ gs_app_set_url (app, ++ AS_URL_KIND_HOMEPAGE, ++ pk_details_get_url (details)); ++ } ++ if (gs_app_get_description (app) == NULL) { ++ gs_app_set_description (app, ++ GS_APP_QUALITY_LOWEST, ++ pk_details_get_description (details)); ++ } ++ size += pk_details_get_size (details); ++ break; + } +- if (gs_app_get_url (app, AS_URL_KIND_HOMEPAGE) == NULL) { +- gs_app_set_url (app, +- AS_URL_KIND_HOMEPAGE, +- pk_details_get_url (details)); +- } +- if (gs_app_get_description (app) == NULL) { +- gs_app_set_description (app, +- GS_APP_QUALITY_LOWEST, +- pk_details_get_description (details)); +- } +- size += pk_details_get_size (details); + } + + /* the size is the size of all sources */ +diff --git a/plugins/packagekit/packagekit-common.h b/plugins/packagekit/packagekit-common.h +index 9f523684..0742ea3a 100644 +--- a/plugins/packagekit/packagekit-common.h ++++ b/plugins/packagekit/packagekit-common.h +@@ -30,9 +30,8 @@ void gs_plugin_packagekit_resolve_packages_app (GsPlugin *plugin, + void gs_plugin_packagekit_set_metadata_from_package (GsPlugin *plugin, + GsApp *app, + PkPackage *package); +-GHashTable * gs_plugin_packagekit_details_array_to_hash (GPtrArray *array); + void gs_plugin_packagekit_refine_details_app (GsPlugin *plugin, +- GHashTable *details_collection, ++ GPtrArray *array, + GsApp *app); + void gs_plugin_packagekit_set_packaging_format (GsPlugin *plugin, + GsApp *app); +-- +2.26.2 + diff --git a/gnome-software.spec b/gnome-software.spec index f728c07..d3ef6d9 100644 --- a/gnome-software.spec +++ b/gnome-software.spec @@ -11,13 +11,18 @@ Name: gnome-software Version: 3.38.0 -Release: 1%{?dist} +Release: 2%{?dist} Summary: A software center for GNOME License: GPLv2+ URL: https://wiki.gnome.org/Apps/Software Source0: https://download.gnome.org/sources/gnome-software/3.38/%{name}-%{version}.tar.xz +# Revert an optimization that broke packagekit updates +# https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1061 +# https://bodhi.fedoraproject.org/updates/FEDORA-2020-7f57486c95#comment-1621958 +Patch0: 0001-Revert-packagekit-Avoid-600000-allocations-when-comp.patch + BuildRequires: gcc BuildRequires: gettext BuildRequires: libxslt @@ -206,6 +211,9 @@ desktop-file-validate %{buildroot}%{_datadir}/applications/*.desktop %{_datadir}/gtk-doc/html/gnome-software %changelog +* Mon Sep 14 2020 Kalev Lember - 3.38.0-2 +- Revert an optimization that broke packagekit updates + * Fri Sep 11 2020 Kalev Lember - 3.38.0-1 - Update to 3.38.0