Patchwork [v2,04/27] qom: avoid useless conversions from string to type

login
register
mail settings
Submitter Paolo Bonzini
Date Feb. 4, 2012, 8:02 a.m.
Message ID <1328342577-25732-5-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/139538/
State New
Headers show

Comments

Paolo Bonzini - Feb. 4, 2012, 8:02 a.m.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)
Anthony Liguori - Feb. 6, 2012, 2:14 p.m.
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);

Patch

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;
+
+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);