Remove the refcounted string interning feature

This made very little difference to the PSS, RSS or load time and increased
complexity considerably.

Additionally, intern the locale when parsing XML documents and be more careful
about refcounting known AsRefString objects rather than re-allocating them
each time.

3 files changed, 76 insertions, 72 deletions

diff --git a/libappstream-glib/as-ref-string.c b/libappstream-glib/as-ref-string.c
index df0a70a..bd4164c 100644
--- a/libappstream-glib/as-ref-string.c
+++ b/libappstream-glib/as-ref-string.c
@@ -41,32 +41,39 @@ typedef struct {
 #define AS_REFPTR_TO_HEADER(o)		((AsRefStringHeader *) ((void *) ((guint8 *) o - sizeof (AsRefStringHeader))))
 #define AS_REFPTR_FROM_HEADER(o)	((gpointer) (((guint8 *) o) + sizeof (AsRefStringHeader)))
 
-static void
-as_ref_string_unref_from_str (gchar *str)
-{
-	AsRefStringHeader *hdr = AS_REFPTR_TO_HEADER (str);
-	g_free (hdr);
-}
+/* use the top bit for static mask */
+#define AS_REFPTR_STATIC_MASK		0x80000000
+#define AS_REFPTR_IS_STATIC(hdr)	(hdr->refcnt & AS_REFPTR_STATIC_MASK)
 
 static GHashTable	*as_ref_string_hash = NULL;
 static GMutex		 as_ref_string_mutex;
 
-static GHashTable *
-as_ref_string_get_hash_safe (void)
+/**
+ * as_ref_string_debug_start:
+ *
+ * Starts collection of refcounted string data.
+ *
+ * Since: 0.7.9
+ */
+void
+as_ref_string_debug_start (void)
 {
-	if (as_ref_string_hash == NULL) {
-		/* gpointer to AsRefStringHeader */
-		as_ref_string_hash = g_hash_table_new_full (g_direct_hash,
-							    g_direct_equal,
-							    (GDestroyNotify) as_ref_string_unref_from_str,
-							    NULL);
-	}
-	return as_ref_string_hash;
+	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
+	if (as_ref_string_hash == NULL)
+		as_ref_string_hash = g_hash_table_new (g_direct_hash, g_direct_equal);
 }
 
-static void __attribute__ ((destructor))
-as_ref_string_destructor (void)
+/**
+ * as_ref_string_debug_end:
+ *
+ * Ends collection of refcounted string data.
+ *
+ * Since: 0.7.9
+ */
+void __attribute__ ((destructor))
+as_ref_string_debug_end (void)
 {
+	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
 	g_clear_pointer (&as_ref_string_hash, g_hash_table_unref);
 }
 
@@ -90,6 +97,8 @@ as_ref_string_destructor (void)
  * Returns a deep copied refcounted string. The returned string can be modified
  * without affecting other refcounted versions.
  *
+ * This function is deprecated since 0.7.9.
+ *
  * Returns: a %AsRefString
  *
  * Since: 0.6.6
@@ -97,22 +106,7 @@ as_ref_string_destructor (void)
 AsRefString *
 as_ref_string_new_copy_with_length (const gchar *str, gsize len)
 {
-	AsRefStringHeader *hdr;
-	AsRefString *rstr_new;
-	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
-
-	/* create object */
-	hdr = g_malloc (len + sizeof (AsRefStringHeader) + 1);
-	hdr->refcnt = 1;
-	rstr_new = AS_REFPTR_FROM_HEADER (hdr);
-	memcpy (rstr_new, str, len);
-	rstr_new[len] = '\0';
-
-	/* add */
-	g_hash_table_add (as_ref_string_get_hash_safe (), rstr_new);
-
-	/* return to data, not the header */
-	return rstr_new;
+	return as_ref_string_new_with_length (str, len);
 }
 
 /**
@@ -122,6 +116,8 @@ as_ref_string_new_copy_with_length (const gchar *str, gsize len)
  * Returns a deep copied refcounted string. The returned string can be modified
  * without affecting other refcounted versions.
  *
+ * This function is deprecated since 0.7.9.
+ *
  * Returns: a %AsRefString
  *
  * Since: 0.6.6
@@ -130,7 +126,7 @@ AsRefString *
 as_ref_string_new_copy (const gchar *str)
 {
 	g_return_val_if_fail (str != NULL, NULL);
-	return as_ref_string_new_copy_with_length (str, strlen (str));
+	return as_ref_string_new_with_length (str, strlen (str));
 }
 
 /**
@@ -148,22 +144,25 @@ as_ref_string_new_copy (const gchar *str)
 AsRefString *
 as_ref_string_new_with_length (const gchar *str, gsize len)
 {
+	g_return_val_if_fail (str != NULL, NULL);
 	AsRefStringHeader *hdr;
-	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
+	AsRefString *rstr_new;
 
-	g_return_val_if_fail (str != NULL, NULL);
+	/* create object */
+	hdr = g_malloc (len + sizeof (AsRefStringHeader) + 1);
+	hdr->refcnt = 1;
+	rstr_new = AS_REFPTR_FROM_HEADER (hdr);
+	memcpy (rstr_new, str, len);
+	rstr_new[len] = '\0';
 
-	/* already in hash */
-	if (g_hash_table_contains (as_ref_string_get_hash_safe (), str)) {
-		hdr = AS_REFPTR_TO_HEADER (str);
-		if (hdr->refcnt < 0)
-			return (AsRefString *) str;
-		g_atomic_int_inc (&hdr->refcnt);
-		return (AsRefString *) str;
+	/* for dedupe stats */
+	if (as_ref_string_hash != NULL) {
+		g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
+		g_hash_table_add (as_ref_string_hash, rstr_new);
 	}
 
-	g_clear_pointer (&locker, g_mutex_locker_free);
-	return as_ref_string_new_copy_with_length (str, len);
+	/* return to data, not the header */
+	return rstr_new;
 }
 
 /**
@@ -200,7 +199,7 @@ as_ref_string_ref (AsRefString *rstr)
 	AsRefStringHeader *hdr;
 	g_return_val_if_fail (rstr != NULL, NULL);
 	hdr = AS_REFPTR_TO_HEADER (rstr);
-	if (hdr->refcnt < 0)
+	if (AS_REFPTR_IS_STATIC (hdr))
 		return rstr;
 	g_atomic_int_inc (&hdr->refcnt);
 	return rstr;
@@ -224,11 +223,17 @@ as_ref_string_unref (AsRefString *rstr)
 	g_return_val_if_fail (rstr != NULL, NULL);
 
 	hdr = AS_REFPTR_TO_HEADER (rstr);
-	if (hdr->refcnt < 0)
+	if (AS_REFPTR_IS_STATIC (hdr))
 		return rstr;
 	if (g_atomic_int_dec_and_test (&hdr->refcnt)) {
-		g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
-		g_hash_table_remove (as_ref_string_get_hash_safe (), rstr);
+
+		/* for dedupe stats */
+		if (as_ref_string_hash != NULL) {
+			g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
+			g_hash_table_remove (as_ref_string_hash, rstr);
+		}
+
+		g_free (hdr);
 		return NULL;
 	}
 	return rstr;
@@ -315,19 +320,21 @@ as_ref_string_sort_by_refcnt_cb (gconstpointer a, gconstpointer b)
 gchar *
 as_ref_string_debug (AsRefStringDebugFlags flags)
 {
-	GHashTable *hash = NULL;
 	GString *tmp = g_string_new (NULL);
 	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&as_ref_string_mutex);
 
+	/* not yet enabled */
+	if (as_ref_string_hash == NULL)
+		return NULL;
+
 	/* overview */
-	hash = as_ref_string_get_hash_safe ();
 	g_string_append_printf (tmp, "Size of hash table: %u\n",
-				g_hash_table_size (hash));
+				g_hash_table_size (as_ref_string_hash));
 
 	/* success: deduped */
 	if (flags & AS_REF_STRING_DEBUG_DEDUPED) {
 		GList *l;
-		g_autoptr(GList) keys = g_hash_table_get_keys (hash);
+		g_autoptr(GList) keys = g_hash_table_get_keys (as_ref_string_hash);
 
 		/* split up sections */
 		if (tmp->len > 0)
@@ -339,7 +346,7 @@ as_ref_string_debug (AsRefStringDebugFlags flags)
 		for (l = keys; l != NULL; l = l->next) {
 			const gchar *str = l->data;
 			AsRefStringHeader *hdr = AS_REFPTR_TO_HEADER (str);
-			if (hdr->refcnt <= 1)
+			if (AS_REFPTR_IS_STATIC (hdr))
 				continue;
 			g_string_append_printf (tmp, "%i\t%s\n", hdr->refcnt, str);
 		}
@@ -350,7 +357,7 @@ as_ref_string_debug (AsRefStringDebugFlags flags)
 		GList *l;
 		GList *l2;
 		g_autoptr(GHashTable) dupes = g_hash_table_new (g_direct_hash, g_direct_equal);
-		g_autoptr(GList) keys = g_hash_table_get_keys (hash);
+		g_autoptr(GList) keys = g_hash_table_get_keys (as_ref_string_hash);
 
 		/* split up sections */
 		if (tmp->len > 0)
@@ -362,6 +369,9 @@ as_ref_string_debug (AsRefStringDebugFlags flags)
 			AsRefStringHeader *hdr = AS_REFPTR_TO_HEADER (str);
 			guint dupe_cnt = 0;
 
+			if (AS_REFPTR_IS_STATIC (hdr))
+				continue;
+
 			if (g_hash_table_contains (dupes, hdr))
 				continue;
 			g_hash_table_add (dupes, (gpointer) hdr);
@@ -369,6 +379,8 @@ as_ref_string_debug (AsRefStringDebugFlags flags)
 			for (l2 = l; l2 != NULL; l2 = l2->next) {
 				const gchar *str2 = l2->data;
 				AsRefStringHeader *hdr2 = AS_REFPTR_TO_HEADER (str2);
+				if (AS_REFPTR_IS_STATIC (hdr2))
+					continue;
 				if (g_hash_table_contains (dupes, hdr2))
 					continue;
 				if (l == l2)
@@ -378,7 +390,7 @@ as_ref_string_debug (AsRefStringDebugFlags flags)
 				g_hash_table_add (dupes, (gpointer) hdr2);
 				dupe_cnt += 1;
 			}
-			if (dupe_cnt > 0) {
+			if (dupe_cnt > 1) {
 				g_string_append_printf (tmp, "%u\t%s\n",
 							dupe_cnt, str);
 			}
diff --git a/libappstream-glib/as-ref-string.h b/libappstream-glib/as-ref-string.h
index 8217247..a0a625f 100644
--- a/libappstream-glib/as-ref-string.h
+++ b/libappstream-glib/as-ref-string.h
@@ -53,9 +53,6 @@ typedef enum {
 AsRefString	*as_ref_string_new			(const gchar	*str);
 AsRefString	*as_ref_string_new_with_length		(const gchar	*str,
 							 gsize		 len);
-AsRefString	*as_ref_string_new_copy			(const gchar	*str);
-AsRefString	*as_ref_string_new_copy_with_length	(const gchar	*str,
-							 gsize		 len);
 AsRefString	*as_ref_string_ref			(AsRefString	*rstr);
 AsRefString	*as_ref_string_unref			(AsRefString	*rstr);
 void		 as_ref_string_assign			(AsRefString	**rstr_ptr,
@@ -63,6 +60,13 @@ void		 as_ref_string_assign			(AsRefString	**rstr_ptr,
 void		 as_ref_string_assign_safe		(AsRefString	**rstr_ptr,
 							 const gchar	*str);
 gchar		*as_ref_string_debug			(AsRefStringDebugFlags flags);
+void		 as_ref_string_debug_start		(void);
+void		 as_ref_string_debug_stop		(void);
+
+/* deprecated */
+AsRefString	*as_ref_string_new_copy			(const gchar	*str);
+AsRefString	*as_ref_string_new_copy_with_length	(const gchar	*str,
+							 gsize		 len);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(AsRefString, as_ref_string_unref)
 
diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index c7b4832..7e61fb5 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -5581,9 +5581,7 @@ as_test_app_parse_data_func (void)
 static void
 as_test_ref_string_func (void)
 {
-	const gchar *tmp;
 	AsRefString *rstr;
-	AsRefString *rstr2;
 
 	/* basic refcounting */
 	rstr = as_ref_string_new ("test");
@@ -5592,16 +5590,6 @@ as_test_ref_string_func (void)
 	g_assert (as_ref_string_ref (rstr) != NULL);
 	g_assert (as_ref_string_unref (rstr) != NULL);
 	g_assert (as_ref_string_unref (rstr) == NULL);
-
-	/* adopting const string */
-	tmp = "test";
-	rstr = as_ref_string_new (tmp);
-	g_assert_cmpstr (rstr, ==, tmp);
-	rstr2 = as_ref_string_new (rstr);
-	g_assert_cmpstr (rstr2, ==, tmp);
-	g_assert (rstr == rstr2);
-	g_assert (as_ref_string_unref (rstr) != NULL);
-	g_assert (as_ref_string_unref (rstr2) == NULL);
 }
 
 int