All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel <qemu-devel@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Andreas Färber" <afaerber@suse.de>,
	"Anthony Liguori" <aliguori@us.ibm.com>
Subject: Re: [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases
Date: Sat, 11 May 2013 09:59:44 +0200	[thread overview]
Message-ID: <518DFA70.3090404@redhat.com> (raw)
In-Reply-To: <8738tu5taq.fsf@codemonkey.ws>

Il 11/05/2013 01:06, Anthony Liguori ha scritto:
> The thing is, none of these casts should be for more than 1 level and
> the patch I provided makes those casts almost free.

Assuming strings are collapsed (and that's why my original attempt to
introduce the fast path only looked at the concrete classes: I was
trying to optimize it within a device model).

> 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.

No, it doesn't.  GCC can do that if you use link-time optimization, but
binutils doesn't look into the data section.  It is a black box.

>>> 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.

Fair enough, thanks!  Are you going to replace patch 3 with yours?
(Though I don't think we need to check the parent, see above).

> 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.

Let's please test virtio-net as well, it looks like Aurelien has the
benchmark.  /me goes swimming now. :)

Paolo

  reply	other threads:[~2013-05-11  8:00 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-10 12:16 [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 1/9] qom: improve documentation of cast functions Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 2/9] qom: allow casting of a NULL class Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 3/9] qom: add a fast path to object_class_dynamic_cast Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 4/9] qom: pass file/line/function to asserting casts Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 5/9] qom: trace " Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 6/9] qom: allow turning cast debugging off Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 7/9] build: disable QOM cast debugging for official releases Paolo Bonzini
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 8/9] qom: simplify object_class_dynamic_cast, part 1 Paolo Bonzini
2013-05-10 17:52   ` Anthony Liguori
2013-05-10 12:16 ` [Qemu-devel] [PATCH for-1.5 9/9] qom: simplify object_class_dynamic_cast, part 2 Paolo Bonzini
2013-05-10 13:01 ` [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases Anthony Liguori
2013-05-10 13:08   ` Paolo Bonzini
2013-05-10 13:23     ` Andreas Färber
2013-05-10 13:30       ` Paolo Bonzini
2013-05-10 14:22         ` Aurelien Jarno
2013-05-10 14:39         ` Anthony Liguori
2013-05-10 14:59           ` Paolo Bonzini
2013-05-10 17:41             ` Anthony Liguori
2013-05-10 19:00               ` Aurelien Jarno
2013-05-10 19:35                 ` Anthony Liguori
2013-05-10 20:59               ` Paolo Bonzini
2013-05-10 23:06                 ` Anthony Liguori
2013-05-11  7:59                   ` Paolo Bonzini [this message]
2013-05-11  7:23                 ` Aurelien Jarno
2013-05-10 14:27     ` Anthony Liguori
2013-05-10 14:46       ` Aurelien Jarno
2013-05-10 15:04         ` Andreas Färber
2013-05-10 15:56 ` Aurelien Jarno
2013-05-10 16:18   ` Andreas Färber
2013-05-10 16:40     ` Paolo Bonzini
2013-05-10 18:50   ` Anthony Liguori
2013-05-13 16:46 ` Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=518DFA70.3090404@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=afaerber@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=aurelien@aurel32.net \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.