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

[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 <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