From f74e52a3a0c441f8e56b796d99c492b1a0a2072c Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 28 Apr 2017 16:06:40 +0100 Subject: [PATCH] libostree: Rework OstreeAsyncProgress to use GVariants internally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OstreeAsyncProgress currently does some contortions to try and avoid allocating space for guints and guint64s (on 64-bit platforms), but this means it uses two GHashTables. A GHashTable allocates 8 buckets even when empty. Given that the largest usage of OstreeAsyncProgress in libostree puts 13 uints and 5 uint64s in it, this optimisation does not save significant (if any) memory. Instead, change OstreeAsyncProgress to store values internally as GVariants, and expose this with some new API: • ostree_async_progress_get_variant() • ostree_async_progress_set_variant() Each GVariant is allocated on the heap. As they are immutable, they are thread-safe once returned by a getter. The existing API continues to work as before, except in the case where a key is set/got as both a uint and a uint64 — there will now be a collision (and a GVariant type checking failure) whereas previously there was no collision. Nothing in OSTree uses OstreeAsyncProgress this way though. The new API can be used to share more complex data via the progress API. Signed-off-by: Philip Withnall Closes: #819 Approved by: cgwalters --- apidoc/ostree-sections.txt | 2 + src/libostree/libostree.sym | 8 +- src/libostree/ostree-async-progress.c | 129 ++++++++++++++------------ src/libostree/ostree-async-progress.h | 7 ++ 4 files changed, 81 insertions(+), 65 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index e6ed4c8a9c..5625a1936d 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -4,9 +4,11 @@ OstreeAsyncProgress ostree_async_progress_new ostree_async_progress_new_and_connect ostree_async_progress_get_status +ostree_async_progress_get_variant ostree_async_progress_get_uint ostree_async_progress_get_uint64 ostree_async_progress_set_status +ostree_async_progress_set_variant ostree_async_progress_set_uint ostree_async_progress_set_uint64 ostree_async_progress_finish diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index d9339bf239..0787d87722 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -394,13 +394,11 @@ global: * NOTE NOTE NOTE */ -/* Uncomment when adding the first new symbol */ -/* -LIBOSTREE_2017.$NEWVERSION { +LIBOSTREE_2017.6 { global: - someostree_symbol_deleteme; + ostree_async_progress_get_variant; + ostree_async_progress_set_variant; } LIBOSTREE_2017.4; -*/ /* Stub section for the stable release *after* this development one; don't * edit this other than to update the last number. This is just a copy/paste diff --git a/src/libostree/ostree-async-progress.c b/src/libostree/ostree-async-progress.c index 0851fd8673..68f4c8349f 100644 --- a/src/libostree/ostree-async-progress.c +++ b/src/libostree/ostree-async-progress.c @@ -22,6 +22,8 @@ #include "ostree-async-progress.h" +#include "libglnx.h" + /** * SECTION:ostree-async-progress * @title: Progress notification system for asynchronous operations @@ -39,12 +41,6 @@ * operation. */ -#if GLIB_SIZEOF_VOID_P == 8 -#define _OSTREE_HAVE_LP64 1 -#else -#define _OSTREE_HAVE_LP64 0 -#endif - enum { CHANGED, LAST_SIGNAL @@ -59,8 +55,7 @@ struct OstreeAsyncProgress GMutex lock; GMainContext *maincontext; GSource *idle_source; - GHashTable *uint_values; - GHashTable *uint64_values; + GHashTable *values; /* (element-type uint GVariant) */ gboolean dead; @@ -79,8 +74,7 @@ ostree_async_progress_finalize (GObject *object) g_mutex_clear (&self->lock); g_clear_pointer (&self->maincontext, g_main_context_unref); g_clear_pointer (&self->idle_source, g_source_unref); - g_hash_table_unref (self->uint_values); - g_hash_table_unref (self->uint64_values); + g_hash_table_unref (self->values); g_free (self->status); G_OBJECT_CLASS (ostree_async_progress_parent_class)->finalize (object); @@ -114,46 +108,53 @@ ostree_async_progress_init (OstreeAsyncProgress *self) { g_mutex_init (&self->lock); self->maincontext = g_main_context_ref_thread_default (); - self->uint_values = g_hash_table_new (NULL, NULL); -#if _OSTREE_HAVE_LP64 - self->uint64_values = g_hash_table_new (NULL, NULL); -#else - self->uint64_values = g_hash_table_new_full (NULL, NULL, - NULL, g_free); -#endif + self->values = g_hash_table_new_full (NULL, NULL, NULL, (GDestroyNotify) g_variant_unref); } -guint -ostree_async_progress_get_uint (OstreeAsyncProgress *self, - const char *key) +/** + * ostree_async_progress_get_variant: + * @self: an #OstreeAsyncProgress + * @key: a key to look up + * + * Look up a key in the #OstreeAsyncProgress and return the #GVariant associated + * with it. The lookup is thread-safe. + * + * Returns: (transfer full) (nullable): value for the given @key, or %NULL if + * it was not set + * Since: 2017.6 + */ +GVariant * +ostree_async_progress_get_variant (OstreeAsyncProgress *self, + const char *key) { - guint rval; + GVariant *rval; + + g_return_val_if_fail (OSTREE_IS_ASYNC_PROGRESS (self), NULL); + g_return_val_if_fail (key != NULL, NULL); + g_mutex_lock (&self->lock); - rval = GPOINTER_TO_UINT (g_hash_table_lookup (self->uint_values, - GUINT_TO_POINTER (g_quark_from_string (key)))); + rval = g_hash_table_lookup (self->values, GUINT_TO_POINTER (g_quark_from_string (key))); + if (rval != NULL) + g_variant_ref (rval); g_mutex_unlock (&self->lock); + return rval; } +guint +ostree_async_progress_get_uint (OstreeAsyncProgress *self, + const char *key) +{ + g_autoptr(GVariant) rval = ostree_async_progress_get_variant (self, key); + return (rval != NULL) ? g_variant_get_uint32 (rval) : 0; +} + guint64 ostree_async_progress_get_uint64 (OstreeAsyncProgress *self, const char *key) { -#if _OSTREE_HAVE_LP64 - guint64 rval; - g_mutex_lock (&self->lock); - rval = (guint64) g_hash_table_lookup (self->uint64_values, GUINT_TO_POINTER (g_quark_from_string (key))); - g_mutex_unlock (&self->lock); - return rval; -#else - guint64 *rval; - g_mutex_lock (&self->lock); - rval = g_hash_table_lookup (self->uint64_values, (gpointer)g_quark_from_string (key)); - g_mutex_unlock (&self->lock); - if (rval) - return *rval; - return 0; -#endif + g_autoptr(GVariant) rval = ostree_async_progress_get_variant (self, key); + return (rval != NULL) ? g_variant_get_uint64 (rval) : 0; } static gboolean @@ -204,26 +205,45 @@ ostree_async_progress_get_status (OstreeAsyncProgress *self) return ret; } -static void -update_key (OstreeAsyncProgress *self, - GHashTable *hash, - const char *key, - gpointer value) +/** + * ostree_async_progress_set_variant: + * @self: an #OstreeAsyncProgress + * @key: a key to set + * @value: the value to assign to @key + * + * Assign a new @value to the given @key, replacing any existing value. The + * operation is thread-safe. @value may be a floating reference; + * g_variant_ref_sink() will be called on it. + * + * Any watchers of the #OstreeAsyncProgress will be notified of the change if + * @value differs from the existing value for @key. + * + * Since: 2017.6 + */ +void +ostree_async_progress_set_variant (OstreeAsyncProgress *self, + const char *key, + GVariant *value) { - gpointer orig_value; + GVariant *orig_value; + g_autoptr(GVariant) new_value = g_variant_ref_sink (value); gpointer qkey = GUINT_TO_POINTER (g_quark_from_string (key)); + g_return_if_fail (OSTREE_IS_ASYNC_PROGRESS (self)); + g_return_if_fail (key != NULL); + g_return_if_fail (value != NULL); + g_mutex_lock (&self->lock); if (self->dead) goto out; - if (g_hash_table_lookup_extended (hash, qkey, NULL, &orig_value)) + if (g_hash_table_lookup_extended (self->values, qkey, NULL, (gpointer *) &orig_value)) { - if (orig_value == value) + if (g_variant_equal (orig_value, new_value)) goto out; } - g_hash_table_replace (hash, qkey, value); + g_hash_table_replace (self->values, qkey, g_steal_pointer (&new_value)); ensure_callback_locked (self); out: @@ -235,7 +255,7 @@ ostree_async_progress_set_uint (OstreeAsyncProgress *self, const char *key, guint value) { - update_key (self, self->uint_values, key, GUINT_TO_POINTER (value)); + ostree_async_progress_set_variant (self, key, g_variant_new_uint32 (value)); } void @@ -243,18 +263,7 @@ ostree_async_progress_set_uint64 (OstreeAsyncProgress *self, const char *key, guint64 value) { - gpointer valuep; - -#if _OSTREE_HAVE_LP64 - valuep = (gpointer)value; -#else - { - guint64 *boxed = g_malloc (sizeof (guint64)); - *boxed = value; - valuep = boxed; - } -#endif - update_key (self, self->uint64_values, key, valuep); + ostree_async_progress_set_variant (self, key, g_variant_new_uint64 (value)); } /** diff --git a/src/libostree/ostree-async-progress.h b/src/libostree/ostree-async-progress.h index ae0e5faab5..62bdcff87b 100644 --- a/src/libostree/ostree-async-progress.h +++ b/src/libostree/ostree-async-progress.h @@ -59,6 +59,9 @@ guint ostree_async_progress_get_uint (OstreeAsyncProgress *self, _OSTREE_PUBLIC guint64 ostree_async_progress_get_uint64 (OstreeAsyncProgress *self, const char *key); +_OSTREE_PUBLIC +GVariant *ostree_async_progress_get_variant (OstreeAsyncProgress *self, + const char *key); _OSTREE_PUBLIC void ostree_async_progress_set_status (OstreeAsyncProgress *self, @@ -72,6 +75,10 @@ _OSTREE_PUBLIC void ostree_async_progress_set_uint64 (OstreeAsyncProgress *self, const char *key, guint64 value); +_OSTREE_PUBLIC +void ostree_async_progress_set_variant (OstreeAsyncProgress *self, + const char *key, + GVariant *value); _OSTREE_PUBLIC void ostree_async_progress_finish (OstreeAsyncProgress *self);