[Qemu-devel] [PATCH v2] qom: object_property_add() performance improvement

[Pavel Fedin]

The function originally behaves very badly when adding properties with "[*]"
suffix. Normally 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 takes 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.
The property is a uint32_t and has the original name of the array ('name[*]')
for simplicity. It has getter function in order to allow to inspect it from
within monitor.
The modification decreases qemu startup time with 32 CPUs by a factor of 2
(~10 sec vs ~20 sec).

Signed-off-by: Pavel Fedin <address@hidden>
---
Changes since v1:
- Fixed typos in comments
- Fixed code style
- Got rid of g_malloc0() for property storage space, now use 'opaque' field 
itself as a
data. Getter is still present for possible inspection from outside (e. g. 
monitor).
---
 qom/object.c | 95 +++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 27 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 96abd34..63773e4 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,66 @@ object_property_add(Object *obj, const char *name, const 
char *type,
     return prop;
 }
 
+static void property_get_uint32_opaque(Object *obj, Visitor *v,
+                                   void *opaque, const char *name,
+                                   Error **errp)
+{
+    uint32_t value = (uintptr_t)opaque;
+    visit_type_uint32(v, &value, name, 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);
+    char *name_no_array;
+    ObjectProperty *ret, *count;
+    int i;
+
+    if (name_len < 3 || memcmp(&name[name_len - 3], "[*]", 4)) {
+        return object_property_add_single(obj, name, type,
+                                          get, set, release, opaque, errp);
+
+    }
+
+    /* 10 characters for maximum possible integer number */
+    name_no_array = g_malloc(name_len + 10);
+
+    count = object_property_find(obj, name, NULL);
+    if (count == NULL) {
+        /* This is very similar to object_property_add_uint32_ptr(), but:
+         * - Returns pointer
+         * - Will not recurse here so that we can use raw name with [*]
+         * - Allows us to use 'opaque' pointer itself as a storage, because
+         *   we want to store only a single integer which should not be
+         *   modified from outside.
+         */
+        count = object_property_add_single(obj, name, "uint32",
+                                           property_get_uint32_opaque, NULL,
+                                           NULL, NULL, &error_abort);
+    }
+
+    name_len -= 3;
+    memcpy(name_no_array, name, name_len);
+    i = (uintptr_t)count->opaque;
+
+    do {
+        g_sprintf(&name_no_array[name_len], "[%d]", i++);
+
+        ret = object_property_add_single(obj, name_no_array, type, get, set,
+                                         release, opaque, NULL);
+    } while (!ret);
+
+    count->opaque = (void *)(intptr_t)i;
+    g_free(name_no_array);
+    return ret;
+}
+
 ObjectProperty *object_property_find(Object *obj, const char *name,
                                      Error **errp)
 {
-- 
1.9.5.msysgit.0