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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 04 Jun 2015 10:35:17 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello Luiz! Have you missed this ?

Well, this is a QOM patch. QOM also has an object abstraction, but
it's not the same as QObject.

I'm CC'ing the maintainer.

> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 
> 
> > -----Original Message-----
> > From: qemu-devel-bounces+p.fedin=samsung.com@nongnu.org [mailto:qemu-devel-
> > bounces+p.fedin=samsung.com@nongnu.org] On Behalf Of Pavel Fedin
> > Sent: Thursday, May 28, 2015 11:42 AM
> > To: qemu-devel@nongnu.org
> > 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 <p.fedin@samsung.com>
> > ---
> >  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
> > 
> 
>