[Qemu-devel] Ping: [PATCH] qobject: object_property_add() performance improvement

[Pavel Fedin]

 Hello Luiz! Have you missed this ?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -----Original Message-----
> From: address@hidden [mailto:qemu-devel-
> address@hidden On Behalf Of Pavel Fedin
> Sent: Thursday, May 28, 2015 11:42 AM
> To: address@hidden
> Cc: 'Luiz Capitulino'
> Subject: [Qemu-devel] [PATCH] qobject: object_property_add() performance 
> improvement
> 
>  The function originally behaves very badly when adding properties with "[*]" 
> suffix.
> Nomnally these are used for numbering IRQ pins. In order to find the correct 
> starting
> number the function started from zero and checked for duplicates. This took 
> incredibly
> long time with large number of CPUs because number of IRQ pins on some 
> architectures
(like
> ARM GICv3) gets multiplied by number of CPUs.
>  The solution is to add one more property which caches last used index so that
duplication
> check is not repeated thousands of times. Every time an array is expanded the 
> index is
> picked up from this cache. Cache property is a uint32_t and has the original 
> name of the
> array ('name[*]') for simplicity.
>  Some more improvements:
> - Call object_property_add_single() instead of recursing into itself - keeps 
> off
memcmp()
> check every time when its result is already known.
> - Allocate name_no_array only once and not every time for every property 
> (there can be
> thousands of them)
>  The modification decreases qemu startup time with 32 ARMv8 CPUs by a factor 
> of 2 (~10
sec
> vs ~20 sec).
> 
> Signed-off-by: Pavel Fedin <address@hidden>
> ---
>  qom/object.c | 89 
> ++++++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 62 insertions(+), 27 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index b8dff43..72480bc 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -10,6 +10,8 @@
>   * See the COPYING file in the top-level directory.
>   */
> 
> +#include <glib/gprintf.h>
> +
>  #include "qom/object.h"
>  #include "qemu-common.h"
>  #include "qapi/visitor.h"
> @@ -721,35 +723,14 @@ void object_unref(Object *obj)
>      }
>  }
> 
> -ObjectProperty *
> -object_property_add(Object *obj, const char *name, const char *type,
> -                    ObjectPropertyAccessor *get,
> -                    ObjectPropertyAccessor *set,
> -                    ObjectPropertyRelease *release,
> -                    void *opaque, Error **errp)
> +static ObjectProperty *
> +object_property_add_single(Object *obj, const char *name, const char *type,
> +                           ObjectPropertyAccessor *get,
> +                           ObjectPropertyAccessor *set,
> +                           ObjectPropertyRelease *release,
> +                           void *opaque, Error **errp)
>  {
>      ObjectProperty *prop;
> -    size_t name_len = strlen(name);
> -
> -    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
> -        int i;
> -        ObjectProperty *ret;
> -        char *name_no_array = g_strdup(name);
> -
> -        name_no_array[name_len - 3] = '\0';
> -        for (i = 0; ; ++i) {
> -            char *full_name = g_strdup_printf("%s[%d]", name_no_array, i);
> -
> -            ret = object_property_add(obj, full_name, type, get, set,
> -                                      release, opaque, NULL);
> -            g_free(full_name);
> -            if (ret) {
> -                break;
> -            }
> -        }
> -        g_free(name_no_array);
> -        return ret;
> -    }
> 
>      QTAILQ_FOREACH(prop, &obj->properties, node) {
>          if (strcmp(prop->name, name) == 0) {
> @@ -774,6 +755,60 @@ object_property_add(Object *obj, const char *name, const 
> char
*type,
>      return prop;
>  }
> 
> +static void property_get_uint32_ptr(Object *obj, Visitor *v,
> +                                   void *opaque, const char *name,
> +                                   Error **errp);
> +
> +ObjectProperty *
> +object_property_add(Object *obj, const char *name, const char *type,
> +                    ObjectPropertyAccessor *get,
> +                    ObjectPropertyAccessor *set,
> +                    ObjectPropertyRelease *release,
> +                    void *opaque, Error **errp)
> +{
> +    size_t name_len = strlen(name);
> +
> +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
> +        int i;
> +        ObjectProperty *ret, *count;
> +        /* 10 characters for maximum possible integer number */
> +        char *name_no_array = g_malloc(name_len + 10);
> +
> +        count = object_property_find(obj, name, NULL);
> +     if (count == NULL) {
> +         void *v = g_malloc0(sizeof(uint32_t));
> +
> +            /* This is the same as object_property_add_uint32_ptr(), but:
> +             * - Slightly faster and returns pointer
> +             * - Will not recurse here so that we can use
> +             *   raw name with [*] here */
> +            count = object_property_add_single(obj, name, "uint32",
> +                                               property_get_uint32_ptr, NULL,
> +                                               NULL, v, &error_abort);
> +     }
> +
> +        name_len -= 2;
> +        memcpy(name_no_array, name, name_len);
> +
> +        for (i = *((uint32_t *)count->opaque); ; ++i) {
> +         g_sprintf(&name_no_array[name_len], "%d]", i);
> +
> +            ret = object_property_add_single(obj, name_no_array, type, get, 
> set,
> +                                             release, opaque, NULL);
> +            if (ret) {
> +                break;
> +            }
> +        }
> +        *((uint32_t *)count->opaque) = i + 1;
> +
> +        g_free(name_no_array);
> +        return ret;
> +    }
> +
> +    return object_property_add_single(obj, name, type,
> +                                      get, set, release, opaque, errp);
> +}
> +
>  ObjectProperty *object_property_find(Object *obj, const char *name,
>                                       Error **errp)
>  {
> --
> 1.9.5.msysgit.0
>