Paolo Bonzini <pbonz...@redhat.com> writes: > Il 10/05/2013 19:41, Anthony Liguori ha scritto: >> Paolo Bonzini <pbonz...@redhat.com> 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
