[Qemu-devel] [PATCH v3] 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>
Reviewed-by: Peter Crosthwaite <address@hidden>
---
 Changes since v2:
- Fixed line spacing
- 'i' is now uintptr_t, got rid of double-casting from void *. String space is 
enlarged to
20 characters in order to accommodate maximum possible 64-bit value.
---
 qom/object.c | 93 ++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 27 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 96abd34..e8b94fb 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,64 @@ 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;
+    uintptr_t i;
+
+    if (name_len < 3 || memcmp(&name[name_len - 3], "[*]", 4)) {
+        return object_property_add_single(obj, name, type,
+                                          get, set, release, opaque, errp);
+    }
+
+    /* 20 characters for maximum possible uintptr_t (64-bit) */
+    name_no_array = g_malloc(name_len + 20);
+
+    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], "[%zu]", i++);
+
+        ret = object_property_add_single(obj, name_no_array, type, get, set,
+                                         release, opaque, NULL);
+    } while (!ret);
+
+    count->opaque = (void *)i;
+    g_free(name_no_array);
+    return ret;
+}
+
 ObjectProperty *object_property_find(Object *obj, const char *name,
                                      Error **errp)
 {
-- 
1.9.5.msysgit.0