From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkCkq-00077V-Pg for qemu-devel@nongnu.org; Thu, 08 Oct 2015 11:05:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkCkn-0002lI-35 for qemu-devel@nongnu.org; Thu, 08 Oct 2015 11:05:12 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:19490) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkCkm-0002g7-PZ for qemu-devel@nongnu.org; Thu, 08 Oct 2015 11:05:08 -0400 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout3.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NVW00M6WPWHP4B0@mailout3.w1.samsung.com> for qemu-devel@nongnu.org; Thu, 08 Oct 2015 16:05:05 +0100 (BST) From: Pavel Fedin References: <1444313344-16196-1-git-send-email-berrange@redhat.com> <1444313344-16196-5-git-send-email-berrange@redhat.com> In-reply-to: <1444313344-16196-5-git-send-email-berrange@redhat.com> Date: Thu, 08 Oct 2015 18:05:02 +0300 Message-id: <00c901d101da$b7003f00$2500bd00$@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: quoted-printable Content-language: ru Subject: Re: [Qemu-devel] [PATCH v3 4/5] qom: replace object property list with GHashTable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "'Daniel P. Berrange'" , qemu-devel@nongnu.org Cc: =?UTF-8?Q?'Andreas_F=C3=A4rber'?= Hello! > -----Original Message----- > From: Daniel P. Berrange [mailto:berrange@redhat.com] > Sent: Thursday, October 08, 2015 5:09 PM > To: qemu-devel@nongnu.org > Cc: Andreas F=C3=A4rber; Pavel Fedin; Daniel P. Berrange > Subject: [PATCH v3 4/5] qom: replace object property list with = GHashTable >=20 > From: Pavel Fedin >=20 > ARM GICv3 systems with large number of CPUs create lots of IRQ pins. = Since > every pin is represented as a property, number of these properties = becomes > very large. Every property add first makes sure there's no duplicates. > Traversing the list becomes very slow, therefore qemu initialization = takes > significant time (several seconds for e. g. 16 CPUs). >=20 > This patch replaces list with GHashTable, making lookup very fast. The = only > drawback is that object_child_foreach() and = object_child_foreach_recursive() > cannot modify their objects during traversal, since GHashTableIter = does not > have modify-safe version. However, the code seems not to modify = objects via > these functions. >=20 > Signed-off-by: Daniel P. Berrange > --- > include/qom/object.h | 10 ++++-- > qom/object.c | 94 = +++++++++++++++++++++++++++++++--------------------- > 2 files changed, 63 insertions(+), 41 deletions(-) >=20 > diff --git a/include/qom/object.h b/include/qom/object.h > index 71503af..7645f5f 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -344,8 +344,6 @@ typedef struct ObjectProperty > ObjectPropertyResolve *resolve; > ObjectPropertyRelease *release; > void *opaque; > - > - QTAILQ_ENTRY(ObjectProperty) node; > } ObjectProperty; >=20 > /** > @@ -405,7 +403,7 @@ struct Object > /*< private >*/ > ObjectClass *class; > ObjectFree *free; > - QTAILQ_HEAD(, ObjectProperty) properties; > + GHashTable *properties; > uint32_t ref; > Object *parent; > }; > @@ -1511,6 +1509,9 @@ void object_property_set_description(Object = *obj, const char *name, > * Call @fn passing each child of @obj and @opaque to it, until @fn = returns > * non-zero. > * > + * It is forbidden to add or remove children from @obj from the @fn > + * callback. > + * > * Returns: The last value returned by @fn, or 0 if there is no = child. > */ > int object_child_foreach(Object *obj, int (*fn)(Object *child, void = *opaque), > @@ -1526,6 +1527,9 @@ int object_child_foreach(Object *obj, int = (*fn)(Object *child, void > *opaque), > * non-zero. Calls recursively, all child nodes of @obj will also be = passed > * all the way down to the leaf nodes of the tree. Depth first = ordering. > * > + * It is forbidden to add or remove children from @obj (or its > + * child nodes) from the @fn callback. > + * > * Returns: The last value returned by @fn, or 0 if there is no = child. > */ > int object_child_foreach_recursive(Object *obj, > diff --git a/qom/object.c b/qom/object.c > index f8b27af..7cec1c9 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -326,6 +326,16 @@ static void object_post_init_with_type(Object = *obj, TypeImpl *ti) > } > } >=20 > +static void property_free(gpointer data) > +{ > + ObjectProperty *prop =3D data; > + > + g_free(prop->name); > + g_free(prop->type); > + g_free(prop->description); > + g_free(prop); > +} > + > void object_initialize_with_type(void *data, size_t size, TypeImpl = *type) > { > Object *obj =3D data; > @@ -340,7 +350,8 @@ void object_initialize_with_type(void *data, = size_t size, TypeImpl *type) > memset(obj, 0, type->instance_size); > obj->class =3D type->class; > object_ref(obj); > - QTAILQ_INIT(&obj->properties); > + obj->properties =3D g_hash_table_new_full(g_str_hash, = g_str_equal, > + NULL, property_free); > object_init_with_type(obj, type); > object_post_init_with_type(obj, type); > } > @@ -359,29 +370,35 @@ static inline bool = object_property_is_child(ObjectProperty *prop) >=20 > static void object_property_del_all(Object *obj) > { > - while (!QTAILQ_EMPTY(&obj->properties)) { > - ObjectProperty *prop =3D QTAILQ_FIRST(&obj->properties); > - > - QTAILQ_REMOVE(&obj->properties, prop, node); > + ObjectProperty *prop; > + GHashTableIter iter; > + gpointer key, value; >=20 > + g_hash_table_iter_init(&iter, obj->properties); > + while (g_hash_table_iter_next(&iter, &key, &value)) { > + prop =3D value; > if (prop->release) { > prop->release(obj, prop->name, prop->opaque); > } > - > - g_free(prop->name); > - g_free(prop->type); > - g_free(prop->description); > - g_free(prop); > } > + > + g_hash_table_unref(obj->properties); > } >=20 > static void object_property_del_child(Object *obj, Object *child, = Error **errp) > { > ObjectProperty *prop; > + GHashTableIter iter; > + gpointer key, value; >=20 > - QTAILQ_FOREACH(prop, &obj->properties, node) { > + g_hash_table_iter_init(&iter, obj->properties); > + while (g_hash_table_iter_next(&iter, &key, &value)) { > + prop =3D value; > if (object_property_is_child(prop) && prop->opaque =3D=3D = child) { > - object_property_del(obj, prop->name, errp); > + if (prop->release) { > + prop->release(obj, prop->name, prop->opaque); > + } > + g_hash_table_iter_remove(&iter); > break; > } > } > @@ -779,10 +796,12 @@ static int do_object_child_foreach(Object *obj, > int (*fn)(Object *child, void = *opaque), > void *opaque, bool recurse) > { > - ObjectProperty *prop, *next; > + GHashTableIter iter; > + ObjectProperty *prop; > int ret =3D 0; >=20 > - QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) { > + g_hash_table_iter_init(&iter, obj->properties); > + while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) { > if (object_property_is_child(prop)) { > Object *child =3D prop->opaque; >=20 > @@ -879,13 +898,11 @@ object_property_add(Object *obj, const char = *name, const char *type, > return ret; > } >=20 > - QTAILQ_FOREACH(prop, &obj->properties, node) { > - if (strcmp(prop->name, name) =3D=3D 0) { > - error_setg(errp, "attempt to add duplicate property '%s'" > + if (g_hash_table_contains(obj->properties, name)) { > + error_setg(errp, "attempt to add duplicate property '%s'" > " to object (type '%s')", name, > object_get_typename(obj)); > - return NULL; > - } > + return NULL; > } >=20 > prop =3D g_malloc0(sizeof(*prop)); > @@ -898,7 +915,7 @@ object_property_add(Object *obj, const char *name, = const char *type, > prop->release =3D release; > prop->opaque =3D opaque; >=20 > - QTAILQ_INSERT_TAIL(&obj->properties, prop, node); > + g_hash_table_insert(obj->properties, prop->name, prop); > return prop; > } >=20 > @@ -907,10 +924,9 @@ ObjectProperty *object_property_find(Object *obj, = const char *name, > { > ObjectProperty *prop; >=20 > - QTAILQ_FOREACH(prop, &obj->properties, node) { > - if (strcmp(prop->name, name) =3D=3D 0) { > - return prop; > - } > + prop =3D g_hash_table_lookup(obj->properties, name); > + if (prop) { > + return prop; > } >=20 > error_setg(errp, "Property '.%s' not found", name); > @@ -922,11 +938,13 @@ void object_property_foreach(Object *obj, > Error **errp, > void *opaque) > { > - ObjectProperty *prop; > Error *local_err =3D NULL; > + GHashTableIter props; > + gpointer key, value; >=20 > - QTAILQ_FOREACH(prop, &obj->properties, node) { > - iter(obj, prop, &local_err, opaque); > + g_hash_table_iter_init(&props, obj->properties); > + while (g_hash_table_iter_next(&props, &key, &value)) { > + iter(obj, value, &local_err, opaque); > if (local_err) { > error_propagate(errp, local_err); > return; > @@ -936,21 +954,17 @@ void object_property_foreach(Object *obj, >=20 > void object_property_del(Object *obj, const char *name, Error **errp) > { > - ObjectProperty *prop =3D object_property_find(obj, name, errp); > - if (prop =3D=3D NULL) { > + ObjectProperty *prop =3D g_hash_table_lookup(obj->properties, = name); > + > + if (!prop) { > + error_setg(errp, "Property '.%s' not found", name); > return; > } >=20 > if (prop->release) { > prop->release(obj, name, prop->opaque); > } > - > - QTAILQ_REMOVE(&obj->properties, prop, node); > - > - g_free(prop->name); > - g_free(prop->type); > - g_free(prop->description); > - g_free(prop); > + g_hash_table_remove(obj->properties, name); > } >=20 > void object_property_get(Object *obj, Visitor *v, const char *name, > @@ -1470,11 +1484,13 @@ void object_property_add_const_link(Object = *obj, const char *name, > gchar *object_get_canonical_path_component(Object *obj) > { > ObjectProperty *prop =3D NULL; > + GHashTableIter iter; >=20 > g_assert(obj); > g_assert(obj->parent !=3D NULL); >=20 > - QTAILQ_FOREACH(prop, &obj->parent->properties, node) { > + g_hash_table_iter_init(&iter, obj->parent->properties); > + while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) { > if (!object_property_is_child(prop)) { > continue; > } > @@ -1558,11 +1574,13 @@ static Object = *object_resolve_partial_path(Object *parent, > bool *ambiguous) > { > Object *obj; > + GHashTableIter iter; > ObjectProperty *prop; >=20 > obj =3D object_resolve_abs_path(parent, parts, typename, 0); >=20 > - QTAILQ_FOREACH(prop, &parent->properties, node) { > + g_hash_table_iter_init(&iter, parent->properties); > + while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&prop)) { > Object *found; >=20 > if (!object_property_is_child(prop)) { > -- > 2.4.3 Signed-off-by: Pavel Fedin Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia