Re: [PATCH v2 04/12] i386: Register feature bit properties as class properties

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, Dec 15, 2020 at 03:11:06PM +0100, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > Class properties make QOM introspection simpler and easier, as
>> > they don't require an object to be instantiated.
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> 
>> This is significantly more than just "simpler and easier".
>> 
>> The other day, I played with the QMP core to reduce its appetite for
>> malloc.  I came up with patches that should approximately halve it, and
>> felt quite pleased with myself.  I looked for a simple test to
>> demonstrate the effect.  Something with plenty of output.  Hmm, why not
>> query-cpu-definitions, it produces about 32KiB.  Instrument, instrument,
>> run, ... whaaaat?!?
>> 
>> My patches save some 7000 allocations (670 KiB total), roughly matching
>> my expectations.
>> 
>> Turns out this is a drop in the bucket: query-cpu-definitions still
>> takes some 180,000 allocations (almost 12 MiB total).  They're hiding
>> behind this line in qmp_query_cpu_definitions():
>> 
>>     g_slist_foreach(list, x86_cpu_definition_entry, &cpu_list);
>> 
>> The line takes more than a quarter second for me.
>> 
>> Hogging the main loop for a quarter second is not good.
>
> Wow!
>
>> 
>> Eduardo's patch reduces run time to 0.02 seconds (40,000 allocations, 9
>> MiB total).  It's a smaller pig now.
>
> Thanks for investigating this!  I'll amend the commit message with:
>
>   Also, the hundreds of instance properties were having an impact
>   on QMP commands that create temporary CPU objects.  On my
>   machine, run time of qmp_query_cpu_definitions() patch changed
>   from ~200ms to ~16ms after applying this patch.
>
> Numbers were obtained with:
>
>  $ sudo perf probe -v -x ./qemu-system-x86_64 -a 
> 'qmp_query_cpu_definitions%return' -a 'qmp_query_cpu_definitions'
>  $ echo -e '{"execute": "qmp_capabilities"}\n{"execute": 
> "query-cpu-definitions"}\n{"execute": "quit"}' | sudo perf trace -e 
> 'probe_qemu:*' ./qemu-system-x86_64 -S -display none -qmp stdio > /dev/null
>
> Before:
>
>      0.000 qemu-system-x8/3103211 
> probe_qemu:qmp_query_cpu_definitions(__probe_ip: 94851767056275)
>    204.072 qemu-system-x8/3103211 
> probe_qemu:qmp_query_cpu_definitions__return(__probe_func: 94851767056275, 
> __probe_ret_ip: 94851768499362)
>
> After:
>
>      0.000 qemu-system-x8/3105969 
> probe_qemu:qmp_query_cpu_definitions(__probe_ip: 94554723186579)
>     16.445 qemu-system-x8/3105969 
> probe_qemu:qmp_query_cpu_definitions__return(__probe_func: 94554723186579, 
> __probe_ret_ip: 94554724629631)

Looks good, thanks!