[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debuggin

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH for-1.5 0/9] Disable expensive QOM cast debugging for official releases
Date: Fri, 10 May 2013 18:06:21 -0500
User-agent: Notmuch/0.15.2+77~g661dcf8 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu)

Paolo Bonzini <address@hidden> writes:

> Il 10/05/2013 19:41, Anthony Liguori ha scritto:
>> Paolo Bonzini <address@hidden> 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

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.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]