From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38074) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UawOh-0004qT-Jb for qemu-devel@nongnu.org; Fri, 10 May 2013 19:06:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UawOg-0007Ms-HB for qemu-devel@nongnu.org; Fri, 10 May 2013 19:06:43 -0400 Received: from e23smtp01.au.ibm.com ([202.81.31.143]:48087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UawOf-0007Mo-VV for qemu-devel@nongnu.org; Fri, 10 May 2013 19:06:42 -0400 Received: from /spool/local by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 11 May 2013 08:58:53 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id 588DC2CE8051 for ; Sat, 11 May 2013 09:06:32 +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 r4AN6McH22085764 for ; Sat, 11 May 2013 09:06:25 +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 r4AN6T3f010301 for ; Sat, 11 May 2013 09:06:29 +1000 From: Anthony Liguori In-Reply-To: <518D5FAA.5050507@redhat.com> References: <1368188203-3407-1-git-send-email-pbonzini@redhat.com> <874nebdm4r.fsf@codemonkey.ws> <518CF130.2030402@redhat.com> <518CF4C4.6020901@suse.de> <518CF669.30803@redhat.com> <871u9ezypj.fsf@codemonkey.ws> <518D0B68.505@redhat.com> <87ip2q91ho.fsf@codemonkey.ws> <518D5FAA.5050507@redhat.com> Date: Fri, 10 May 2013 18:06:21 -0500 Message-ID: <8738tu5taq.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: mst@redhat.com, qemu-devel@nongnu.org, Aurelien Jarno , Andreas =?utf-8?Q?F=C3=A4rber?= Paolo Bonzini writes: > Il 10/05/2013 19:41, Anthony Liguori ha scritto: >> Paolo Bonzini writes: >> >>> Il 10/05/2013 16:39, Anthony Liguori ha scritto: >>>> I just oppose the notion of disabling casts and *especially* only >>>> disabling casts for official builds. >>> >>> This actually happens all the time. Exactly this kind of type-safe cast >>> is disabled in releases of GCC, but enabled when building from svn trunk. >> >> Let's assume for a moment that you are right and this behavior is what >> we should have. Let's also assume there is a real regression here >> which has yet to have been established. > > Aurelien timed the effect of my patch two hours before you sent this > message. If it's not a regression in 1.5 (which is quite obvious from > the profile), it is a regression from the introduction of CPU classes > (1.3 or 1.4), when this code didn't exist at all. > > And in 1.5 we introduced virtio-net casts as well (or did mst sneak in > his change anyway? ;)). If 10% is the effect of a few hundred > interrupts/sec, perhaps the same effect is visible on a few thousand > packets/sec. I wouldn't bet against that one week from release. The thing is, none of these casts should be for more than 1 level and the patch I provided makes those casts almost free. I believe the reason it didn't fix the problem for Aurelien is because the string addresses were different because of different compilation units. I guess binutils doesn't collapse strings when linking. >> As soon as we open up 1.6 for development, we face an immediate >> regression in performance. So we need to fix the real problem here >> anyway. > > No, we don't. We will simply have to live with a debugging vs. > production tradeoff. I appreciate all of the arguments below. I'm very concerned about reducing safety checks but am sympathetic to performance concerns. Here's what I would like to do. I'll apply 1-6 of your series which gives us the infrastructure to disable qom casts. That at least let's the code get tested in case we need it. I will hold off applying patch 7 hoping that the patch I provided to Aurelien solves the problem. If it doesn't and we're unable to find a better solution, I'll apply patch 7 for the release. Either way, the infrastructure is there for a distro to make a decision although I think disabling qom casts is an extremely bad decision to make. Given the v2 version of my patch, I'm quite confident that casting in the vast majority of circumstances would avoid a g_hash_table lookup so that should eliminate any performance concerns. Regards, Anthony Liguori