[gnome-software] odrs: Reduce size of critical sections
- From: Richard Hughes <rhughes src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software] odrs: Reduce size of critical sections
- Date: Fri, 12 Jun 2020 14:13:11 +0000 (UTC)
commit 0968391a344968171d648cab894135c7f6a6dba8
Author: Philip Withnall <withnall endlessm com>
Date: Mon Jun 1 12:38:46 2020 +0100
odrs: Reduce size of critical sections
Previously, the ODRS mutex was being held while a 780KB JSON file
(`ratings.json`) was being parsed and processed. In another place in the
ODRS plugin, the mutex was being acquired and released in a loop.
Neither are conducive to good performance.
Reduce the size/frequency of the critical sections by moving the mutex
locking out of a loop, and atomically replacing the entire ratings array
when refreshing it.
Signed-off-by: Philip Withnall <withnall endlessm com>
plugins/odrs/gs-plugin-odrs.c | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
---
diff --git a/plugins/odrs/gs-plugin-odrs.c b/plugins/odrs/gs-plugin-odrs.c
index 17f2aab3..c8dca5e0 100644
--- a/plugins/odrs/gs-plugin-odrs.c
+++ b/plugins/odrs/gs-plugin-odrs.c
@@ -27,7 +27,7 @@ struct GsPluginData {
gchar *distro;
gchar *user_hash;
gchar *review_server;
- GHashTable *ratings;
+ GHashTable *ratings; /* (mutex ratings_mutex) (owned) (nullable) */
GMutex ratings_mutex;
GsApp *cached_origin;
};
@@ -43,8 +43,7 @@ gs_plugin_initialize (GsPlugin *plugin)
priv->settings = g_settings_new ("org.gnome.software");
priv->review_server = g_settings_get_string (priv->settings,
"review-server");
- priv->ratings = g_hash_table_new_full (g_str_hash, g_str_equal,
- g_free, (GDestroyNotify) g_array_unref);
+ priv->ratings = NULL; /* until first refreshed */
/* get the machine+user ID hash value */
priv->user_hash = gs_utils_get_user_hash (&error);
@@ -113,10 +112,11 @@ gs_plugin_odrs_load_ratings (GsPlugin *plugin, const gchar *fn, GError **error)
JsonObject *json_item;
g_autoptr(GList) apps = NULL;
g_autoptr(JsonParser) json_parser = NULL;
- g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->ratings_mutex);
+ g_autoptr(GHashTable) new_ratings = NULL;
+ g_autoptr(GMutexLocker) locker = NULL;
- /* remove all existing */
- g_hash_table_remove_all (priv->ratings);
+ new_ratings = g_hash_table_new_full (g_str_hash, g_str_equal,
+ g_free, (GDestroyNotify) g_array_unref);
/* parse the data and find the success */
json_parser = json_parser_new ();
@@ -149,11 +149,17 @@ gs_plugin_odrs_load_ratings (GsPlugin *plugin, const gchar *fn, GError **error)
g_autoptr(GArray) ratings = NULL;;
ratings = gs_plugin_odrs_load_ratings_for_app (json_app);
if (ratings->len == 6) {
- g_hash_table_insert (priv->ratings,
+ g_hash_table_insert (new_ratings,
g_strdup (app_id),
g_array_ref (ratings));
}
}
+
+ /* Update the shared state */
+ locker = g_mutex_locker_new (&priv->ratings_mutex);
+ g_clear_pointer (&priv->ratings, g_hash_table_unref);
+ priv->ratings = g_steal_pointer (&new_ratings);
+
return TRUE;
}
@@ -219,7 +225,7 @@ gs_plugin_destroy (GsPlugin *plugin)
g_free (priv->user_hash);
g_free (priv->distro);
g_free (priv->review_server);
- g_hash_table_unref (priv->ratings);
+ g_clear_pointer (&priv->ratings, g_hash_table_unref);
g_object_unref (priv->settings);
g_object_unref (priv->cached_origin);
g_mutex_clear (&priv->ratings_mutex);
@@ -514,12 +520,18 @@ gs_plugin_odrs_refine_ratings (GsPlugin *plugin,
guint cnt = 0;
g_autoptr(GArray) review_ratings = NULL;
g_autoptr(GPtrArray) reviewable_ids = NULL;
+ g_autoptr(GMutexLocker) locker = NULL;
/* get ratings for each reviewable ID */
reviewable_ids = _gs_app_get_reviewable_ids (app);
+
+ locker = g_mutex_locker_new (&priv->ratings_mutex);
+
+ if (priv->ratings == NULL)
+ return TRUE;
+
for (guint i = 0; i < reviewable_ids->len; i++) {
const gchar *id = g_ptr_array_index (reviewable_ids, i);
- g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->ratings_mutex);
GArray *ratings_tmp = g_hash_table_lookup (priv->ratings, id);
if (ratings_tmp == NULL)
continue;
@@ -531,6 +543,9 @@ gs_plugin_odrs_refine_ratings (GsPlugin *plugin,
if (cnt == 0)
return TRUE;
+ /* Done with priv->ratings now */
+ g_clear_pointer (&locker, g_mutex_locker_free);
+
/* merge to accumulator array back to one GArray blob */
review_ratings = g_array_sized_new (FALSE, TRUE, sizeof(guint32), 6);
for (guint i = 0; i < 6; i++)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]