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

[Pavel Fedin]

 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