Commit 793a4f2c authored by Richard Hughes's avatar Richard Hughes

Remove the hash table in GsAppList

The as_utils_unique_id_hash() functions do not work well when the unique_id is
changed at runtime, as GsApps are allowed to do. This removes some of the
'strangeness' that can be seen when refining wildcard applications.
parent 1f95742f
Pipeline #36128 passed with stage
in 3 minutes and 30 seconds
......@@ -42,7 +42,6 @@ struct _GsAppList
{
GObject parent_instance;
GPtrArray *array;
GHashTable *hash_by_id; /* app-id : app */
GMutex mutex;
guint size_peak;
GsAppListFlags flags;
......@@ -236,6 +235,17 @@ gs_app_list_get_size_peak (GsAppList *list)
return list->size_peak;
}
static GsApp *
gs_app_list_lookup_safe (GsAppList *list, const gchar *unique_id)
{
for (guint i = 0; i < list->array->len; i++) {
GsApp *app = g_ptr_array_index (list->array, i);
if (as_utils_unique_id_equal (gs_app_get_unique_id (app), unique_id))
return app;
}
return NULL;
}
/**
* gs_app_list_lookup:
* @list: A #GsAppList
......@@ -252,7 +262,7 @@ GsApp *
gs_app_list_lookup (GsAppList *list, const gchar *unique_id)
{
g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&list->mutex);
return g_hash_table_lookup (list->hash_by_id, unique_id);
return gs_app_list_lookup_safe (list, unique_id);
}
/**
......@@ -302,7 +312,6 @@ gs_app_list_check_for_duplicate (GsAppList *list, GsApp *app)
{
GsApp *app_old;
const gchar *id;
const gchar *id_old = NULL;
/* adding a wildcard */
if (gs_app_has_quirk (app, GS_APP_QUIRK_IS_WILDCARD)) {
......@@ -332,26 +341,14 @@ gs_app_list_check_for_duplicate (GsAppList *list, GsApp *app)
return TRUE;
}
app_old = g_hash_table_lookup (list->hash_by_id, id);
if (app_old == NULL)
return TRUE;
/* existing app is a wildcard */
id_old = gs_app_get_unique_id (app_old);
if (gs_app_has_quirk (app_old, GS_APP_QUIRK_IS_WILDCARD)) {
g_debug ("adding %s as %s is a wildcard", id, id_old);
app_old = gs_app_list_lookup_safe (list, id);
if (app_old == NULL)
return TRUE;
}
/* do a sanity check */
if (!as_utils_unique_id_equal (id, id_old)) {
g_debug ("unique-id non-equal %s as %s but hash matched!",
id, id_old);
if (gs_app_has_quirk (app_old, GS_APP_QUIRK_IS_WILDCARD))
return TRUE;
}
/* already exists */
g_debug ("not adding duplicate %s as %s already exists", id, id_old);
return FALSE;
}
......@@ -382,7 +379,6 @@ gs_app_list_add_safe (GsAppList *list, GsApp *app, GsAppListAddFlag flag)
/* just use the ref */
gs_app_list_maybe_watch_app (list, app);
g_ptr_array_add (list->array, g_object_ref (app));
g_hash_table_insert (list->hash_by_id, g_strdup (id), g_object_ref (app));
/* update the historical max */
if (list->array->len > list->size_peak)
......@@ -431,28 +427,14 @@ gs_app_list_add (GsAppList *list, GsApp *app)
void
gs_app_list_remove (GsAppList *list, GsApp *app)
{
GsApp *app_tmp;
const gchar *unique_id;
g_autoptr(GMutexLocker) locker = NULL;
g_return_if_fail (GS_IS_APP_LIST (list));
g_return_if_fail (GS_IS_APP (app));
locker = g_mutex_locker_new (&list->mutex);
/* remove, or ignore if not found */
unique_id = gs_app_get_unique_id (app);
if (unique_id != NULL) {
app_tmp = g_hash_table_lookup (list->hash_by_id, unique_id);
if (app_tmp == NULL)
return;
g_hash_table_remove (list->hash_by_id, unique_id);
g_ptr_array_remove (list->array, app_tmp);
gs_app_list_maybe_unwatch_app (list, app_tmp);
} else {
g_ptr_array_remove (list->array, app);
gs_app_list_maybe_unwatch_app (list, app);
}
g_ptr_array_remove (list->array, app);
gs_app_list_maybe_unwatch_app (list, app);
/* recalculate global state */
gs_app_list_invalidate_state (list);
......@@ -533,7 +515,6 @@ gs_app_list_remove_all_safe (GsAppList *list)
gs_app_list_maybe_unwatch_app (list, app);
}
g_ptr_array_set_size (list->array, 0);
g_hash_table_remove_all (list->hash_by_id);
gs_app_list_invalidate_state (list);
gs_app_list_invalidate_progress (list);
}
......@@ -655,17 +636,6 @@ gs_app_list_truncate (GsAppList *list, guint length)
/* remove the apps in the positions larger than the length */
locker = g_mutex_locker_new (&list->mutex);
for (guint i = length; i < list->array->len; i++) {
GsApp *app = g_ptr_array_index (list->array, i);
const gchar *unique_id;
unique_id = gs_app_get_unique_id (app);
if (unique_id != NULL) {
GsApp *app_tmp = g_hash_table_lookup (list->hash_by_id, unique_id);
if (app_tmp != NULL)
g_hash_table_remove (list->hash_by_id, unique_id);
}
}
g_ptr_array_set_size (list->array, length);
}
......@@ -940,7 +910,6 @@ gs_app_list_finalize (GObject *object)
{
GsAppList *list = GS_APP_LIST (object);
g_ptr_array_unref (list->array);
g_hash_table_unref (list->hash_by_id);
g_mutex_clear (&list->mutex);
G_OBJECT_CLASS (gs_app_list_parent_class)->finalize (object);
}
......@@ -977,10 +946,6 @@ gs_app_list_init (GsAppList *list)
{
g_mutex_init (&list->mutex);
list->array = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
list->hash_by_id = g_hash_table_new_full ((GHashFunc) as_utils_unique_id_hash,
(GEqualFunc) as_utils_unique_id_equal,
g_free,
(GDestroyNotify) g_object_unref);
}
/**
......
......@@ -535,8 +535,6 @@ gs_plugin_func (void)
list = gs_app_list_new ();
app = gs_app_new ("a");
gs_app_list_add (list, app);
g_object_unref (app);
app = gs_app_new ("a");
gs_app_list_remove (list, app);
g_object_unref (app);
g_assert_cmpint (gs_app_list_length (list), ==, 0);
......@@ -770,6 +768,28 @@ gs_app_list_func (void)
g_assert_cmpint (gs_app_list_get_state (list), ==, AS_APP_STATE_UNKNOWN);
}
static void
gs_app_list_performance_func (void)
{
g_autoptr(GPtrArray) apps = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
g_autoptr(GsAppList) list = gs_app_list_new ();
g_autoptr(GTimer) timer = NULL;
/* create a few apps */
for (guint i = 0; i < 500; i++) {
g_autofree gchar *id = g_strdup_printf ("%03u.desktop", i);
g_ptr_array_add (apps, gs_app_new (id));
}
/* add them to the list */
timer = g_timer_new ();
for (guint i = 0; i < apps->len; i++) {
GsApp *app = g_ptr_array_index (apps, i);
gs_app_list_add (list, app);
}
g_print ("%.2fms ", g_timer_elapsed (timer, NULL) * 1000);
}
static void
gs_app_list_related_func (void)
{
......@@ -812,6 +832,7 @@ main (int argc, char **argv)
g_test_add_func ("/gnome-software/lib/app{unique-id}", gs_app_unique_id_func);
g_test_add_func ("/gnome-software/lib/app{thread}", gs_app_thread_func);
g_test_add_func ("/gnome-software/lib/app{list}", gs_app_list_func);
g_test_add_func ("/gnome-software/lib/app{list-performance}", gs_app_list_performance_func);
g_test_add_func ("/gnome-software/lib/app{list-related}", gs_app_list_related_func);
g_test_add_func ("/gnome-software/lib/plugin", gs_plugin_func);
g_test_add_func ("/gnome-software/lib/plugin{download-rewrite}", gs_plugin_download_rewrite_func);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment