qom: object_property_add() performance improvement

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 <[email protected]>

---
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(-)

qom/object.c +8 -27

index 96abd34..63773e4 100644
@@ -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;
 }

qom/object.c +35 -1

+    }
+
+    /* 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

10 secs is still pretty brutal for machine init. Got any clues for where the rest of the time is going?

Just define it as a uintptr_t to avoid the casts.