From patchwork Fri May 10 22:58:23 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Anthony Liguori X-Patchwork-Id: 243092 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 25A2B2C00E3 for ; Sat, 11 May 2013 08:59:08 +1000 (EST) Received: from localhost ([::1]:57270 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UawHK-0003M2-Fm for incoming@patchwork.ozlabs.org; Fri, 10 May 2013 18:59:06 -0400 Received: from eggs.gnu.org ([208.118.235.92]:36328) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UawH2-0003Lu-M1 for qemu-devel@nongnu.org; Fri, 10 May 2013 18:58:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UawH1-00058r-58 for qemu-devel@nongnu.org; Fri, 10 May 2013 18:58:48 -0400 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:33095) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UawH0-00058Z-K8 for qemu-devel@nongnu.org; Fri, 10 May 2013 18:58:47 -0400 Received: from /spool/local by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 11 May 2013 08:53:16 +1000 Received: from d23dlp03.au.ibm.com (202.81.31.214) by e23smtp05.au.ibm.com (202.81.31.211) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Sat, 11 May 2013 08:53:14 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id B88F33578018 for ; Sat, 11 May 2013 08:58:31 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4AMwOBb17432814 for ; Sat, 11 May 2013 08:58:24 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4AMwUhV001221 for ; Sat, 11 May 2013 08:58:30 +1000 Received: from titi.na.relay.ibm.com (sig-9-48-118-123.mts.ibm.com [9.48.118.123]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id r4AMwOtS001170; Sat, 11 May 2013 08:58:28 +1000 From: Anthony Liguori To: Aurelien Jarno In-Reply-To: <20130510214740.GN31148@hall.aurel32.net> References: <1368211675-2912-1-git-send-email-aliguori@us.ibm.com> <20130510214740.GN31148@hall.aurel32.net> User-Agent: Notmuch/0.15.2+77~g661dcf8 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) Date: Fri, 10 May 2013 17:58:23 -0500 Message-ID: <877gj65to0.fsf@codemonkey.ws> MIME-Version: 1.0 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13051022-1396-0000-0000-000002F3FF8A X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 202.81.31.147 Cc: Paolo Bonzini , qemu-devel@nongnu.org, Andreas =?utf-8?Q?F=C3=A4rber?= Subject: Re: [Qemu-devel] [PATCH for-1.5] qom: optimize casting to leaf class and parent class X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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; >> } >> >> 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; >> + } >> + >> + 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 From 269442decf0f137cf7d09c32442fcd16dd319901 Mon Sep 17 00:00:00 2001 From: Anthony Liguori Date: Fri, 10 May 2013 13:43:11 -0500 Subject: [PATCH] qom: optimize casting to leaf class and parent class (v2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- v1 -> v2: - use strcmp() instead of == --- qom/object.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/qom/object.c b/qom/object.c index 75e6aac..99b5b33 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; } static TypeImpl *type_get_by_name(const char *name) @@ -449,10 +455,17 @@ 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 (strcmp(type->name, typename) == 0 || + strcmp(type->parent, typename) == 0) { + return class; + } + + target_type = type_get_by_name(typename); + if (!target_type) { /* target class type unknown, so fail the cast */ return NULL; -- 1.8.0