On Wed, Jun 10, 2015 at 12:51 AM, Pavel Fedin <p.fe...@samsung.com> wrote:
> 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.
blank line between paras.
> 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).\
10 secs is still pretty brutal for machine init. Got any clues for
where the rest of the time is going?
>
> Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
> ---
> 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);
> +}
> +
> +
one less blank.
> +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;
Just define it as a uintptr_t to avoid the casts.
> +
> + if (name_len < 3 || memcmp(&name[name_len - 3], "[*]", 4)) {
> + return object_property_add_single(obj, name, type,
> + get, set, release, opaque, errp);
> +
remove this blank.
otherwise,
Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
Regards,
Peter
> + }
> +
> + /* 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
>
>
>
