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

[Anthony Liguori <aliguori@us.ibm.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 10/05/2013 15:01, Anthony Liguori ha scritto:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> Cast debugging can have a substantial cost (20% or more, measured by
>>> Aurelien on qemu-system-ppc64).
>> 
>> [Needs citation]
>
> Sure: http://permalink.gmane.org/gmane.comp.emulators.qemu/210830
>
>  49,73%  perf-10672.map           [.] 0x7f7853ab4e0f
>  13,23%  qemu-system-ppc64        [.] cpu_ppc_exec
>  13,16%  libglib-2.0.so.0.3200.4  [.] g_hash_table_lookup
>   8,18%  libglib-2.0.so.0.3200.4  [.] g_str_hash
>   2,47%  qemu-system-ppc64        [.] object_class_dynamic_cast
>   1,97%  qemu-system-ppc64        [.] type_is_ancestor
>   1,05%  libglib-2.0.so.0.3200.4  [.] g_str_equal
>
> That's ~27%.
>
>>> Instead of adding special-cased "fast
>>> casts" in the hot paths, we can just disable it in releases.  At the
>>> same time, add tracing facilities that simplify the analysys of those
>>> problems that cast debugging would reveal.
>> 
>> I'd prefer not to disable but instead focus on improving performance.
>
> For 1.5?  This is a regression in 1.5 due to more and more usage of
> foo_env_on_cpu.

If this is a regression, then we should be able to show:

1) workload XYZ gets X in 1.4 and Y in 1.5

2) applying your series demonstrates that we go back to X performance

I'm not convinced this is a true set of statements.  13% of time spent
doing casts could simply suggest that the vcpu is largely idle and has
nothing better to do.

At any rate, given (1), we can at least try some other less invasive
approachs to fixing the problem.  Just removing the unnecessary cast
might end up doing it.

Regards,

Anthony Liguori

>
> Paolo
>
>> I suspect any performance overhead is resolving the type string to
>> typeimpl.  One work around is to have a dynamic cast that takes a
>> typeimpl and then use a function that returns a static similar to how
>> glib works.
>> 
>> If you've got a reproducible case where the overhead is high, it should
>> be easy to check.
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>>>
>>> At least patches 1-7 are for 1.5.
>>>
>>> Paolo Bonzini (9):
>>>   qom: improve documentation of cast functions
>>>   qom: allow casting of a NULL class
>>>   qom: add a fast path to object_class_dynamic_cast
>>>   qom: pass file/line/function to asserting casts
>>>   qom: trace asserting casts
>>>   qom: allow turning cast debugging off
>>>   build: disable QOM cast debugging for official releases
>>>   qom: simplify object_class_dynamic_cast, part 1
>>>   qom: simplify object_class_dynamic_cast, part 2
>>>
>>>  configure            | 20 ++++++++------
>>>  include/qom/object.h | 40 ++++++++++++++++++++++-----
>>>  qom/object.c         | 77 ++++++++++++++++++++++++++++++++++------------------
>>>  trace-events         |  3 ++
>>>  4 files changed, 99 insertions(+), 41 deletions(-)
>>>
>>> -- 
>>> 1.8.1.4
>>