Skip to content

Commit

Permalink
qom: Replace object property list with GHashTable
Browse files Browse the repository at this point in the history 
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).

This patch replaces list with GHashTable, making lookup very fast. The only
drawback is that object_child_foreach() and object_child_foreach_recursive()
cannot add or remove properties during traversal, since GHashTableIter does
not have modify-safe version. However, the code seems not to modify objects
via these functions.

Signed-off-by: Pavel Fedin <[email protected]>
Signed-off-by: Daniel P. Berrange <[email protected]>
Tested-by: Pavel Fedin <[email protected]>
[AF: Fixed object_property_del_{all,child}() issues;
     g_hash_table_contains() -> g_hash_table_lookup(), suggested by Daniel]
Reviewed-by: Daniel P. Berrange <[email protected]>
Signed-off-by: Andreas Färber <[email protected]>
  • Loading branch information
@pfedin @afaerber
pfedin authored and afaerber committed Nov 19, 2015
1 parent 8c4d156 commit b604a85
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 47 deletions.
10 changes: 7 additions & 3 deletions include/qom/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,6 @@ typedef struct ObjectProperty
ObjectPropertyResolve *resolve;
ObjectPropertyRelease *release;
void *opaque;

QTAILQ_ENTRY(ObjectProperty) node;
} ObjectProperty;

/**
Expand Down Expand Up @@ -405,7 +403,7 @@ struct Object
/*< private >*/
ObjectClass *class;
ObjectFree *free;
QTAILQ_HEAD(, ObjectProperty) properties;
GHashTable *properties;
uint32_t ref;
Object *parent;
};
Expand Down Expand Up @@ -1537,6 +1535,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),
Expand All @@ -1552,6 +1553,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,
Expand Down
120 changes: 76 additions & 44 deletions qom/object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ struct TypeImpl
};

struct ObjectPropertyIterator {
ObjectProperty *next;
GHashTableIter iter;
};

static Type type_interface;
Expand Down Expand Up @@ -330,6 +330,16 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti)
}
}

static void object_property_free(gpointer data)
{
ObjectProperty *prop = 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 = data;
Expand All @@ -344,7 +354,8 @@ void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
memset(obj, 0, type->instance_size);
obj->class = type->class;
object_ref(obj);
QTAILQ_INIT(&obj->properties);
obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal,
NULL, object_property_free);
object_init_with_type(obj, type);
object_post_init_with_type(obj, type);
}
Expand All @@ -363,29 +374,51 @@ static inline bool object_property_is_child(ObjectProperty *prop)

static void object_property_del_all(Object *obj)
{
while (!QTAILQ_EMPTY(&obj->properties)) {
ObjectProperty *prop = QTAILQ_FIRST(&obj->properties);

QTAILQ_REMOVE(&obj->properties, prop, node);

if (prop->release) {
prop->release(obj, prop->name, prop->opaque);
ObjectProperty *prop;
GHashTableIter iter;
gpointer key, value;
bool released;

do {
released = false;
g_hash_table_iter_init(&iter, obj->properties);
while (g_hash_table_iter_next(&iter, &key, &value)) {
prop = value;
if (prop->release) {
prop->release(obj, prop->name, prop->opaque);
prop->release = NULL;
released = true;
break;
}
g_hash_table_iter_remove(&iter);
}
} while (released);

g_free(prop->name);
g_free(prop->type);
g_free(prop->description);
g_free(prop);
}
g_hash_table_unref(obj->properties);
}

static void object_property_del_child(Object *obj, Object *child, Error **errp)
{
ObjectProperty *prop;
GHashTableIter iter;
gpointer key, value;

QTAILQ_FOREACH(prop, &obj->properties, node) {
g_hash_table_iter_init(&iter, obj->properties);
while (g_hash_table_iter_next(&iter, &key, &value)) {
prop = value;
if (object_property_is_child(prop) && prop->opaque == child) {
if (prop->release) {
prop->release(obj, prop->name, prop->opaque);
prop->release = NULL;
}
break;
}
}
g_hash_table_iter_init(&iter, obj->properties);
while (g_hash_table_iter_next(&iter, &key, &value)) {
prop = value;
if (object_property_is_child(prop) && prop->opaque == child) {
object_property_del(obj, prop->name, errp);
g_hash_table_iter_remove(&iter);
break;
}
}
Expand Down Expand Up @@ -783,10 +816,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 = 0;

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 = prop->opaque;

Expand Down Expand Up @@ -883,13 +918,11 @@ object_property_add(Object *obj, const char *name, const char *type,
return ret;
}

QTAILQ_FOREACH(prop, &obj->properties, node) {
if (strcmp(prop->name, name) == 0) {
error_setg(errp, "attempt to add duplicate property '%s'"
if (g_hash_table_lookup(obj->properties, name) != NULL) {
error_setg(errp, "attempt to add duplicate property '%s'"
" to object (type '%s')", name,
object_get_typename(obj));
return NULL;
}
return NULL;
}

prop = g_malloc0(sizeof(*prop));
Expand All @@ -902,7 +935,7 @@ object_property_add(Object *obj, const char *name, const char *type,
prop->release = release;
prop->opaque = opaque;

QTAILQ_INSERT_TAIL(&obj->properties, prop, node);
g_hash_table_insert(obj->properties, prop->name, prop);
return prop;
}

Expand All @@ -911,10 +944,9 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
{
ObjectProperty *prop;

QTAILQ_FOREACH(prop, &obj->properties, node) {
if (strcmp(prop->name, name) == 0) {
return prop;
}
prop = g_hash_table_lookup(obj->properties, name);
if (prop) {
return prop;
}

error_setg(errp, "Property '.%s' not found", name);
Expand All @@ -924,7 +956,7 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
ObjectPropertyIterator *object_property_iter_init(Object *obj)
{
ObjectPropertyIterator *ret = g_new0(ObjectPropertyIterator, 1);
ret->next = QTAILQ_FIRST(&obj->properties);
g_hash_table_iter_init(&ret->iter, obj->properties);
return ret;
}

Expand All @@ -938,30 +970,26 @@ void object_property_iter_free(ObjectPropertyIterator *iter)

ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter)
{
ObjectProperty *ret = iter->next;
if (ret) {
iter->next = QTAILQ_NEXT(iter->next, node);
gpointer key, val;
if (!g_hash_table_iter_next(&iter->iter, &key, &val)) {
return NULL;
}
return ret;
return val;
}

void object_property_del(Object *obj, const char *name, Error **errp)
{
ObjectProperty *prop = object_property_find(obj, name, errp);
if (prop == NULL) {
ObjectProperty *prop = g_hash_table_lookup(obj->properties, name);

if (!prop) {
error_setg(errp, "Property '.%s' not found", name);
return;
}

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);
}

void object_property_get(Object *obj, Visitor *v, const char *name,
Expand Down Expand Up @@ -1481,11 +1509,13 @@ void object_property_add_const_link(Object *obj, const char *name,
gchar *object_get_canonical_path_component(Object *obj)
{
ObjectProperty *prop = NULL;
GHashTableIter iter;

g_assert(obj);
g_assert(obj->parent != NULL);

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;
}
Expand Down Expand Up @@ -1569,11 +1599,13 @@ static Object *object_resolve_partial_path(Object *parent,
bool *ambiguous)
{
Object *obj;
GHashTableIter iter;
ObjectProperty *prop;

obj = object_resolve_abs_path(parent, parts, typename, 0);

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;

if (!object_property_is_child(prop)) {
Expand Down

0 comments on commit b604a85

Please sign in to comment.