Re: [Qemu-devel] [PATCH v2 04/27] qom: avoid useless conversions from string to type

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/04/2012 02:02 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   qom/object.c |   16 +++++++++-------
>   1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 4d21f0a..a89e9e3 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -368,9 +368,10 @@ void object_delete(Object *obj)
>       g_free(obj);
>   }
>
> -static bool object_is_type(Object *obj, const char *typename)
> +static Type type_interface;
> +

Statics belong at the top of the file.  But I'd rather not introduce a global 
unless we have some sort of compelling data that shows that it's a performance 
problem.

The type lookup is done through a hash table, at most it's going to be 2-3 
integer comparisons followed by a strcmp() of a small word.  It shouldn't be 
that slow.

Regards,

Anthony Liguori

> +static bool object_is_type(Object *obj, TypeImpl *target_type)
>   {
> -    TypeImpl *target_type = type_get_by_name(typename);
>       TypeImpl *type = obj->class->type;
>
>       /* Check if typename is a direct ancestor of type */
> @@ -387,10 +388,11 @@ static bool object_is_type(Object *obj, const char *typename)
>
>   Object *object_dynamic_cast(Object *obj, const char *typename)
>   {
> +    TypeImpl *target_type = type_get_by_name(typename);
>       GSList *i;
>
>       /* Check if typename is a direct ancestor */
> -    if (object_is_type(obj, typename)) {
> +    if (object_is_type(obj, target_type)) {
>           return obj;
>       }
>
> @@ -404,10 +406,10 @@ Object *object_dynamic_cast(Object *obj, const char *typename)
>        * (run-time) type safety and to speed up tests like this one.  If we ever
>        * do that we can revisit the order here.
>        */
> -    if (object_is_type(obj, TYPE_INTERFACE)) {
> +    if (object_is_type(obj, type_interface)) {
>           assert(!obj->interfaces);
>           obj = INTERFACE(obj)->obj;
> -        if (object_is_type(obj, typename)) {
> +        if (object_is_type(obj, target_type)) {
>               return obj;
>           }
>       }
> @@ -416,7 +418,7 @@ Object *object_dynamic_cast(Object *obj, const char *typename)
>       for (i = obj->interfaces; i; i = i->next) {
>           Interface *iface = i->data;
>
> -        if (object_is_type(OBJECT(iface), typename)) {
> +        if (object_is_type(OBJECT(iface), target_type)) {
>               return OBJECT(iface);
>           }
>       }
> @@ -433,7 +435,7 @@ static void register_interface(void)
>           .abstract = true,
>       };
>
> -    type_register_static(&interface_info);
> +    type_interface = type_register_static(&interface_info);
>   }
>
>   device_init(register_interface);