From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54055) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UbEdc-00040u-SG for qemu-devel@nongnu.org; Sat, 11 May 2013 14:35:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UbEdb-0000qj-Dc for qemu-devel@nongnu.org; Sat, 11 May 2013 14:35:20 -0400 Received: from mail-wi0-x22c.google.com ([2a00:1450:400c:c05::22c]:41867) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UbEdb-0000qZ-4h for qemu-devel@nongnu.org; Sat, 11 May 2013 14:35:19 -0400 Received: by mail-wi0-f172.google.com with SMTP id hm14so1635618wib.5 for ; Sat, 11 May 2013 11:35:18 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <518E8F60.6020704@redhat.com> Date: Sat, 11 May 2013 20:35:12 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1368211675-2912-1-git-send-email-aliguori@us.ibm.com> <20130510214740.GN31148@hall.aurel32.net> <877gj65to0.fsf@codemonkey.ws> In-Reply-To: <877gj65to0.fsf@codemonkey.ws> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH for-1.5] qom: optimize casting to leaf class and parent class List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel , Aurelien Jarno , Anthony Liguori Il 11/05/2013 00:58, Anthony Liguori ha scritto: > Aurelien Jarno writes: > >> On Fri, May 10, 2013 at 01:47:55PM -0500, Anthony Liguori wrote: >>> Most QOM types use type_register_static but we still strdup the >>> passed data. However, the original pointers are useful because >>> GCC is pretty good about collapsing strings so its very likely any >>> use of the pointer will end up being that same address. >>> >>> IOW, with a little trickery, we can compare types by just comparing >>> strings and in fact that's what we do here. >>> >>> We do this for the two most common cases, casting to a leaf class >>> or to the parent class. >>> >>> With these two changes, I see a decrease from around 2 hash table >>> lookups to only a thousand with no run time lookups at all. >>> >>> Cc: Paolo Bonzini >>> Cc: Aurelien Jarno >>> Cc: Andreas Färber >>> Reported-by: Aurelien Jarno >>> Signed-off-by: Anthony Liguori >>> --- >>> Aurelien, could you please try this patch with your PPC test case? >>> --- >>> qom/object.c | 16 ++++++++++++++-- >>> 1 file changed, 14 insertions(+), 2 deletions(-) >>> >>> diff --git a/qom/object.c b/qom/object.c >>> index 75e6aac..5ecfd28 100644 >>> --- a/qom/object.c >>> +++ b/qom/object.c >>> @@ -132,7 +132,13 @@ TypeImpl *type_register(const TypeInfo *info) >>> >>> TypeImpl *type_register_static(const TypeInfo *info) >>> { >>> - return type_register(info); >>> + TypeImpl *impl; >>> + >>> + impl = type_register(info); >>> + impl->name = info->name; >>> + impl->parent = info->parent; >>> + >>> + return impl; >>> } This is ok with a comment. >>> static TypeImpl *type_get_by_name(const char *name) >>> @@ -449,10 +455,16 @@ Object *object_dynamic_cast_assert(Object *obj, const char *typename) >>> ObjectClass *object_class_dynamic_cast(ObjectClass *class, >>> const char *typename) >>> { >>> - TypeImpl *target_type = type_get_by_name(typename); >>> + TypeImpl *target_type; >>> TypeImpl *type = class->type; >>> ObjectClass *ret = NULL; >>> >>> + if (type->name == typename || type->parent == typename) { >>> + return class; >>> + } I prefer my patch 3/9. With the hunk above, it works fine for the simple case of casts in a device model's callbacks (testing type->parent would almost always fail, so it is not worthwhile). Unfortunately, strcmp is just as bad as a hashtable lookup (both are O(n) in the size the string, instead of O(1)). Paolo >>> + target_type = type_get_by_name(typename); >>> + >>> if (!target_type) { >>> /* target class type unknown, so fail the cast */ >>> return NULL; >> >> Unfortunately it doesn't fix the problem. I only see a 0.5% improvement, >> which might be in the noise. I still see g_hash_table_lookup and >> g_str_hash quite high in perf top. > > I was afraid of this. I assume the cast comes somewhere other than > where the type was registered. > > This patch should address that. Could you post an image too? Then I > don't have to keep bugging you with updated patches. > > > > > Regards, > > Anthony Liguori > >> >> -- >> Aurelien Jarno GPG: 1024D/F1BCDB73 >> aurelien@aurel32.net http://www.aurel32.net