[PATCH][RFC]Use a hash table for device properties.

Cornelia Huck cohuck at de.ibm.com
Fri Aug 26 01:01:10 EST 2005


This patch converts the list of properties maintained in HalDevice to a
GHashTable. It manages to get a few seconds off the start time on my system
(about 1:30 min); I guess more savings could be made by implementing a
generic device hash table (I'll look at that next).

While this implementation works for me, please test and review carefully :)

 device.c |  553 ++++++++++++++++++++++++++++++++++-----------------------------
 device.h |    2 
 2 files changed, 304 insertions(+), 251 deletions(-)

Index: hald/device.c
===================================================================
RCS file: /cvs/hal/hal/hald/device.c,v
retrieving revision 1.19
diff -u -r1.19 device.c
--- hald/device.c	29 Jul 2005 20:32:57 -0000	1.19
+++ hald/device.c	25 Aug 2005 14:46:01 -0000
@@ -52,6 +52,12 @@
 int dbg_hal_device_object_delta = 0;
 #endif
 
+static gboolean
+hal_hash_property_free (gpointer key, gpointer value, gpointer user_data)
+{
+	return TRUE;
+}
+
 static void
 hal_device_finalize (GObject *obj)
 {
@@ -62,8 +68,8 @@
 	printf ("************* in finalize for udi=%s\n", device->udi);
 #endif
 
-	g_slist_foreach (device->properties, (GFunc) hal_property_free, NULL);
-
+	g_hash_table_foreach_remove (device->property_hash,
+				     hal_hash_property_free, NULL);
 	g_free (device->udi);
 
 	if (parent_class->finalize)
@@ -126,12 +132,24 @@
 }
 
 static void
+hal_device_property_destroy (gpointer data)
+{
+	HalProperty *p = data;
+
+	hal_property_free (p);
+}
+
+static void
 hal_device_init (HalDevice *device)
 {
 	static int temp_device_counter = 0;
 
 	device->udi = g_strdup_printf ("/org/freedesktop/Hal/devices/temp/%d",
 				       temp_device_counter++);
+
+	device->property_hash =
+		g_hash_table_new_full (g_str_hash, g_str_equal, NULL,
+				       hal_device_property_destroy);
 }
 
 GType
@@ -174,6 +192,81 @@
 	return device;
 }
 
+struct hal_device_merge_with_rewrite_data {
+	HalDevice *target;
+	const char *target_namespace;
+	const char *source_namespace;
+};
+
+static void
+hal_device_merge_with_rewrite_func (gpointer k, gpointer value,
+				    gpointer user_data)
+{
+	HalProperty *p = value;
+	int type;
+	int target_type;
+	const char *key = k;
+	int source_ns_len;
+	struct hal_device_merge_with_rewrite_data *data = user_data;
+	gchar *target_key;
+
+	source_ns_len = strlen(data->source_namespace);
+
+	/* only care about properties that match source namespace */
+	if (strncmp(key, data->source_namespace, source_ns_len) != 0)
+		return;
+
+	target_key = g_strdup_printf("%s%s", data->target_namespace,
+				     key+source_ns_len);
+
+	type = hal_property_get_type (p);
+
+	/* only remove target if it exists with a different type */
+	target_type = hal_device_property_get_type (data->target, key);
+	if (target_type != HAL_PROPERTY_TYPE_INVALID && target_type != type)
+		hal_device_property_remove (data->target, key);
+
+	switch (type) {
+
+	case HAL_PROPERTY_TYPE_STRING:
+		hal_device_property_set_string (
+			data->target, target_key,
+			hal_property_get_string (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_INT32:
+		hal_device_property_set_int (
+			data->target, target_key,
+			hal_property_get_int (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_UINT64:
+		hal_device_property_set_uint64 (
+			data->target, target_key,
+			hal_property_get_uint64 (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_BOOLEAN:
+		hal_device_property_set_bool (
+			data->target, target_key,
+			hal_property_get_bool (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_DOUBLE:
+		hal_device_property_set_double (
+			data->target, target_key,
+			hal_property_get_double (p));
+		break;
+
+	default:
+		HAL_WARNING (("Unknown property type %d", type));
+		break;
+	}
+
+	g_free (target_key);
+
+}
+
 /** Merge all properties from source where the key starts with 
  *  source_namespace and put them onto target replacing source_namespace
  *  with target_namespace
@@ -189,80 +282,73 @@
 				const char   *target_namespace,
 				const char   *source_namespace)
 {
-	GSList *iter;
-	size_t source_ns_len;
-
-	source_ns_len = strlen (source_namespace);
+	struct hal_device_merge_with_rewrite_data data = {
+		.target = target,
+		.target_namespace = target_namespace,
+		.source_namespace = source_namespace,
+	};
 
 	/* doesn't handle info.capabilities */
 
 	/* device_property_atomic_update_begin (); */
 
-	for (iter = source->properties; iter != NULL; iter = iter->next) {
-		HalProperty *p = iter->data;
-		int type;
-		const char *key;
-		int target_type;
-		gchar *target_key;
-
-		key = hal_property_get_key (p);
-
-		/* only care about properties that match source namespace */
-		if (strncmp(key, source_namespace, source_ns_len) != 0)
-			continue;
-
-		target_key = g_strdup_printf("%s%s", target_namespace,
-					     key+source_ns_len);
-
-		type = hal_property_get_type (p);
-
-		/* only remove target if it exists with a different type */
-		target_type = hal_device_property_get_type (target, key);
-		if (target_type != HAL_PROPERTY_TYPE_INVALID && target_type != type)
-			hal_device_property_remove (target, key);
+	g_hash_table_foreach(source->property_hash,
+			     hal_device_merge_with_rewrite_func, &data);
 
-		switch (type) {
-
-		case HAL_PROPERTY_TYPE_STRING:
-			hal_device_property_set_string (
-				target, target_key,
-				hal_property_get_string (p));
-			break;
-
-		case HAL_PROPERTY_TYPE_INT32:
-			hal_device_property_set_int (
-				target, target_key,
-				hal_property_get_int (p));
-			break;
-
-		case HAL_PROPERTY_TYPE_UINT64:
-			hal_device_property_set_uint64 (
-				target, target_key,
-				hal_property_get_uint64 (p));
-			break;
+	/* device_property_atomic_update_end (); */
+}
 
-		case HAL_PROPERTY_TYPE_BOOLEAN:
-			hal_device_property_set_bool (
-				target, target_key,
-				hal_property_get_bool (p));
-			break;
+static void
+hal_device_merge_func (gpointer k, gpointer value, gpointer user_data)
+{
+	HalProperty *p = value;
+	int type;
+	const char *key = k;
+	int target_type;
+	HalDevice *target = user_data;
 
-		case HAL_PROPERTY_TYPE_DOUBLE:
-			hal_device_property_set_double (
-				target, target_key,
-				hal_property_get_double (p));
-			break;
+	type = hal_property_get_type (p);
 
-		default:
-			HAL_WARNING (("Unknown property type %d", type));
-			break;
-		}
+	/* handle info.capabilities in a special way */
+	if (strcmp (key, "info.capabilities") == 0)
+		return;
 
-		g_free (target_key);
+	/* only remove target if it exists with a different type */
+	target_type = hal_device_property_get_type (target, key);
+	if (target_type != HAL_PROPERTY_TYPE_INVALID && target_type != type)
+		hal_device_property_remove (target, key);
+
+	switch (type) {
+
+	case HAL_PROPERTY_TYPE_STRING:
+		hal_device_property_set_string (
+			target, key, hal_property_get_string (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_INT32:
+		hal_device_property_set_int (
+			target, key, hal_property_get_int (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_UINT64:
+		hal_device_property_set_uint64 (
+			target, key, hal_property_get_uint64 (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_BOOLEAN:
+		hal_device_property_set_bool (
+			target, key, hal_property_get_bool (p));
+		break;
+
+	case HAL_PROPERTY_TYPE_DOUBLE:
+		hal_device_property_set_double (
+			target, key, hal_property_get_double (p));
+		break;
+
+	default:
+		HAL_WARNING (("Unknown property type %d", type));
+		break;
 	}
-
-	/* device_property_atomic_update_end (); */
-
 }
 
 void
@@ -273,61 +359,8 @@
 
 	/* device_property_atomic_update_begin (); */
 
-	for (iter = source->properties; iter != NULL; iter = iter->next) {
-		HalProperty *p = iter->data;
-		int type;
-		const char *key;
-		int target_type;
-
-		key = hal_property_get_key (p);
-		type = hal_property_get_type (p);
-
-		/* handle info.capabilities in a special way */
-		if (strcmp (key, "info.capabilities") == 0)
-			continue;
-
-		/* only remove target if it exists with a different type */
-		target_type = hal_device_property_get_type (target, key);
-		if (target_type != HAL_PROPERTY_TYPE_INVALID && target_type != type)
-			hal_device_property_remove (target, key);
-
-		switch (type) {
-
-		case HAL_PROPERTY_TYPE_STRING:
-			hal_device_property_set_string (
-				target, key,
-				hal_property_get_string (p));
-			break;
-
-		case HAL_PROPERTY_TYPE_INT32:
-			hal_device_property_set_int (
-				target, key,
-				hal_property_get_int (p));
-			break;
-
-		case HAL_PROPERTY_TYPE_UINT64:
-			hal_device_property_set_uint64 (
-				target, key,
-				hal_property_get_uint64 (p));
-			break;
-
-		case HAL_PROPERTY_TYPE_BOOLEAN:
-			hal_device_property_set_bool (
-				target, key,
-				hal_property_get_bool (p));
-			break;
-
-		case HAL_PROPERTY_TYPE_DOUBLE:
-			hal_device_property_set_double (
-				target, key,
-				hal_property_get_double (p));
-			break;
-
-		default:
-			HAL_WARNING (("Unknown property type %d", type));
-			break;
-		}
-	}
+	g_hash_table_foreach (source->property_hash, hal_device_merge_func,
+			      target);
 
 	/* device_property_atomic_update_end (); */
 
@@ -338,72 +371,82 @@
 	}
 }
 
-gboolean
-hal_device_matches (HalDevice *device1, HalDevice *device2,
-		    const char *namespace)
-{
-	int len;
-	GSList *iter;
 
-	len = strlen (namespace);
+struct hal_device_matches_data {
+	HalDevice *device;
+	const char *namespace;
+};
 
-	for (iter = device1->properties; iter != NULL; iter = iter->next) {
-		HalProperty *p;
-		const char *key;
-		int type;
+static gboolean
+hal_device_matches_func (gpointer k, gpointer value, gpointer user_data)
+{
+	HalProperty *p = value;
+	const char *key = k;
+	int type;
+	struct hal_device_matches_data *data = user_data;
 
-		p = (HalProperty *) iter->data;
-		key = hal_property_get_key (p);
-		type = hal_property_get_type (p);
+	type = hal_property_get_type (p);
 
-		if (strncmp (key, namespace, len) != 0)
-			continue;
+	if (strncmp (key, data->namespace, strlen(data->namespace)) != 0)
+		return FALSE;
 
-		if (!hal_device_has_property (device2, key))
-			return FALSE;
+	if (!hal_device_has_property (data->device, key))
+		return TRUE;
 
-		switch (type) {
+	switch (type) {
 
-		case HAL_PROPERTY_TYPE_STRING:
-			if (strcmp (hal_property_get_string (p),
-				    hal_device_property_get_string (device2,
-								    key)) != 0)
-				return FALSE;
-			break;
+	case HAL_PROPERTY_TYPE_STRING:
+		if (strcmp (hal_property_get_string (p),
+			    hal_device_property_get_string (data->device,
+							    key)) != 0)
+			return TRUE;
+		break;
 
-		case HAL_PROPERTY_TYPE_INT32:
-			if (hal_property_get_int (p) !=
-			    hal_device_property_get_int (device2, key))
-				return FALSE;
-			break;
+	case HAL_PROPERTY_TYPE_INT32:
+		if (hal_property_get_int (p) !=
+		    hal_device_property_get_int (data->device, key))
+			return TRUE;
+		break;
 
-		case HAL_PROPERTY_TYPE_UINT64:
-			if (hal_property_get_uint64 (p) !=
-				hal_device_property_get_uint64 (device2, key))
-				return FALSE;
-			break;
+	case HAL_PROPERTY_TYPE_UINT64:
+		if (hal_property_get_uint64 (p) !=
+		    hal_device_property_get_uint64 (data->device, key))
+			return TRUE;
+		break;
 
-		case HAL_PROPERTY_TYPE_BOOLEAN:
-			if (hal_property_get_bool (p) !=
-			    hal_device_property_get_bool (device2, key))
-				return FALSE;
-			break;
+	case HAL_PROPERTY_TYPE_BOOLEAN:
+		if (hal_property_get_bool (p) !=
+		    hal_device_property_get_bool (data->device, key))
+			return TRUE;
+		break;
 
-		case HAL_PROPERTY_TYPE_DOUBLE:
-			if (hal_property_get_double (p) !=
-			    hal_device_property_get_double (device2, key))
-				return FALSE;
-			break;
+	case HAL_PROPERTY_TYPE_DOUBLE:
+		if (hal_property_get_double (p) !=
+		    hal_device_property_get_double (data->device, key))
+			return TRUE;
+		break;
 
-		default:
-			HAL_WARNING (("Unknown property type %d", type));
-			break;
-		}
+	default:
+		HAL_WARNING (("Unknown property type %d", type));
+		break;
 	}
+	return FALSE;
+}
 
-	return TRUE;
+gboolean
+hal_device_matches (HalDevice *device1, HalDevice *device2,
+                    const char *namespace)
+{
+	struct hal_device_matches_data data = {
+		.device = device2,
+		.namespace = namespace,
+	};
+
+	return !g_hash_table_find (device1->property_hash,
+				   hal_device_matches_func, &data);
 }
 
+
 const char *
 hal_device_get_udi (HalDevice *device)
 {
@@ -461,25 +504,16 @@
 {
 	g_return_val_if_fail (device != NULL, -1);
 
-	return g_slist_length (device->properties);
+	return g_hash_table_size (device->property_hash);
 }
 
 HalProperty *
 hal_device_property_find (HalDevice *device, const char *key)
 {
-	GSList *iter;
-
 	g_return_val_if_fail (device != NULL, NULL);
 	g_return_val_if_fail (key != NULL, NULL);
 
-	for (iter = device->properties; iter != NULL; iter = iter->next) {
-		HalProperty *p = iter->data;
-
-		if (strcmp (hal_property_get_key (p), key) == 0)
-			return p;
-	}
-
-	return NULL;
+	return (HalProperty *)g_hash_table_lookup (device->property_hash, key);
 }
 
 char *
@@ -494,25 +528,40 @@
 	return hal_property_to_string (prop);
 }
 
+struct hal_property_callback_data {
+	HalDevice *device;
+	HalDevicePropertyForeachFn callback;
+	gpointer user_data;
+};
+
+static gboolean
+hal_device_property_callback (gpointer key, gpointer value, gpointer user_data)
+{
+	struct hal_property_callback_data *data = user_data;
+
+	return (!data->callback (data->device, (HalProperty *)value,
+				 data->user_data));
+}
+
 void
 hal_device_property_foreach (HalDevice *device,
 			     HalDevicePropertyForeachFn callback,
 			     gpointer user_data)
 {
-	GSList *iter;
+	struct hal_property_callback_data data;
 
 	g_return_if_fail (device != NULL);
 	g_return_if_fail (callback != NULL);
 
-	for (iter = device->properties; iter != NULL; iter = iter->next) {
-		HalProperty *p = iter->data;
-		gboolean cont;
+	data = (struct hal_property_callback_data) {
+		.device = device,
+		.callback = callback,
+		.user_data = user_data,
+	};
 
-		cont = callback (device, p, user_data);
+	g_hash_table_find (device->property_hash, hal_device_property_callback,
+			   &data);
 
-		if (cont == FALSE)
-			return;
-	}
 }
 
 int
@@ -702,7 +751,8 @@
 
 		prop = hal_property_new_string (key, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -736,7 +786,8 @@
 	} else {
 		prop = hal_property_new_int (key, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -770,7 +821,8 @@
 	} else {
 		prop = hal_property_new_uint64 (key, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -804,7 +856,8 @@
 	} else {
 		prop = hal_property_new_bool (key, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -838,7 +891,8 @@
 	} else {
 		prop = hal_property_new_double (key, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -885,16 +939,7 @@
 gboolean
 hal_device_property_remove (HalDevice *device, const char *key)
 {
-	HalProperty *prop;
-
-	prop = hal_device_property_find (device, key);
-
-	if (prop == NULL)
-		return FALSE;
-
-	device->properties = g_slist_remove (device->properties, prop);
-
-	hal_property_free (prop);
+	g_hash_table_remove (device->property_hash, key);
 
 	g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 		       key, TRUE, FALSE);
@@ -918,55 +963,60 @@
 	return TRUE;
 }
 
-void
-hal_device_print (HalDevice *device)
-{
-	GSList *iter;
 
-        fprintf (stderr, "device udi = %s\n", hal_device_get_udi (device));
-
-	for (iter = device->properties; iter != NULL; iter = iter->next) {
-		HalProperty *p = iter->data;
-                int type;
-                const char *key;
-
-                key = hal_property_get_key (p);
-                type = hal_property_get_type (p);
-
-                switch (type) {
-                case HAL_PROPERTY_TYPE_STRING:
-                        fprintf (stderr, "  %s = '%s'  (string)\n", key,
-                                hal_property_get_string (p));
-                        break;
+static void
+hal_device_print_func (gpointer k, gpointer value, gpointer user_data)
+{
+	HalProperty *p = value;
+	int type;
+	char *key = k;
+
+	type = hal_property_get_type (p);
+
+	switch (type) {
+	case HAL_PROPERTY_TYPE_STRING:
+		fprintf (stderr, "  %s = '%s'  (string)\n", key,
+			 hal_property_get_string (p));
+		break;
  
-                case HAL_PROPERTY_TYPE_INT32:
-                        fprintf (stderr, "  %s = %d  0x%x  (int)\n", key,
-                                hal_property_get_int (p),
-                                hal_property_get_int (p));
-                        break;
+	case HAL_PROPERTY_TYPE_INT32:
+		fprintf (stderr, "  %s = %d  0x%x  (int)\n", key,
+			 hal_property_get_int (p),
+			 hal_property_get_int (p));
+		break;
  
-                case HAL_PROPERTY_TYPE_UINT64:
-                        fprintf (stderr, "  %s = %lld  0x%llx  (uint64)\n", key,
-                                hal_property_get_uint64 (p),
-                                hal_property_get_uint64 (p));
-                        break;
+	case HAL_PROPERTY_TYPE_UINT64:
+		fprintf (stderr, "  %s = %lld  0x%llx  (uint64)\n", key,
+			 hal_property_get_uint64 (p),
+			 hal_property_get_uint64 (p));
+		break;
  
-                case HAL_PROPERTY_TYPE_DOUBLE:
-                        fprintf (stderr, "  %s = %g  (double)\n", key,
-                                hal_property_get_double (p));
-                        break;
+	case HAL_PROPERTY_TYPE_DOUBLE:
+		fprintf (stderr, "  %s = %g  (double)\n", key,
+			 hal_property_get_double (p));
+		break;
  
-                case HAL_PROPERTY_TYPE_BOOLEAN:
-                        fprintf (stderr, "  %s = %s  (bool)\n", key,
-                                (hal_property_get_bool (p) ? "true" :
-                                 "false"));
-                        break;
+	case HAL_PROPERTY_TYPE_BOOLEAN:
+		fprintf (stderr, "  %s = %s  (bool)\n", key,
+			 (hal_property_get_bool (p) ? "true" :
+			  "false"));
+		break;
  
-                default:
-                        HAL_WARNING (("Unknown property type %d", type));
-                        break;
-                }
-        }
+	default:
+		HAL_WARNING (("Unknown property type %d", type));
+		break;
+	}
+}
+
+
+void
+hal_device_print (HalDevice *device)
+{
+        fprintf (stderr, "device udi = %s\n", hal_device_get_udi (device));
+
+	g_hash_table_foreach (device->property_hash, hal_device_print_func,
+			      NULL);
+
         fprintf (stderr, "\n");
 }
 
@@ -1133,7 +1183,8 @@
 		prop = hal_property_new_strlist (key);
 		hal_property_strlist_append (prop, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -1165,7 +1216,8 @@
 		prop = hal_property_new_strlist (key);
 		hal_property_strlist_prepend (prop, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
@@ -1226,7 +1278,8 @@
 		prop = hal_property_new_strlist (key);
 		hal_property_strlist_prepend (prop, value);
 
-		device->properties = g_slist_prepend (device->properties, prop);
+		g_hash_table_insert (device->property_hash, (gpointer)key,
+				     prop);
 
 		g_signal_emit (device, signals[PROPERTY_CHANGED], 0,
 			       key, FALSE, TRUE);
Index: hald/device.h
===================================================================
RCS file: /cvs/hal/hal/hald/device.h,v
retrieving revision 1.12
diff -u -r1.12 device.h
--- hald/device.h	29 Jul 2005 20:32:57 -0000	1.12
+++ hald/device.h	25 Aug 2005 14:46:01 -0000
@@ -40,7 +40,7 @@
 
 	char *udi;
 	
-	GSList *properties;
+	GHashTable *property_hash;
 };
 
 struct _HalDeviceClass {


More information about the hal mailing list