Il mar 20 mag 2025, 21:01 Stefan Hajnoczi <stefa...@gmail.com> ha scritto:
> On Mon, May 19, 2025 at 2:52 PM Tanish Desai <tanishdesa...@gmail.com>
> wrote:
> >
> > Remove hot paths from .c file and added it in .h file to keep it inline.
>
> Please include performance results in the commit description so it's
> clear what impact this change has.
>
Hi Stefan,
I am replying because I take the blame for this :) and as an example of how
he could interact with the maintainer.
For now we mostly looked at differences between the code that tracetool
generates for the various backends, and the observation that some of them
put code in the .h and some in the .c file. I explained to Tanish the
concept of separating hot vs cold code in theory, showed him some examples
in QEMU where performance measurements were done in the past, and suggested
applying this to various backends (starting with the one with the weirdest
choice!). However we didn't do any measurement yet.
Some possibilities that come to mind:
1) maybe the coroutine benchmarks are enough to show a difference, with
some luck
2) a new microbenchmark (or a set of microbenchmarks that try various level
of register pressure around the tracepoint), which would be nice to have
anyway
3) perhaps we could try to check the code size for some object files in
block/ (for example libblock.a.p/*.c.o), as a proxy for how much
instruction cache space is saved when all tracepoints are disabled
We can start from 3, but also try 1+3 and 2+3 if it fails if you think that
would be more persuasive.
Thanks,
Paolo
>
> > Signed-off-by: Tanish Desai <tanishdesa...@gmail.com>
> > ---
> > scripts/tracetool/backend/simple.py | 23 ++++++++++-------------
>
> Please use scripts/get_maintainer.pl to find the emails to CC:
>
> $ scripts/get_maintainer.pl -f scripts/tracetool/backend/simple.py
> Stefan Hajnoczi <stefa...@redhat.com> (maintainer:Tracing)
> Mads Ynddal <m...@ynddal.dk> (reviewer:Tracing)
> qemu-devel@nongnu.org (open list:All patches CC here)
>
> qemu-devel is a high-traffic list and maintainers may not see your
> patches unless you CC them.
>
> > 1 file changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/scripts/tracetool/backend/simple.py
> b/scripts/tracetool/backend/simple.py
> > index a74d61fcd6..2688d4b64b 100644
> > --- a/scripts/tracetool/backend/simple.py
> > +++ b/scripts/tracetool/backend/simple.py
> > @@ -36,8 +36,17 @@ def generate_h_begin(events, group):
> >
> >
> > def generate_h(event, group):
> > - out(' _simple_%(api)s(%(args)s);',
> > + event_id = 'TRACE_' + event.name.upper()
> > + if "vcpu" in event.properties:
> > + # already checked on the generic format code
> > + cond = "true"
> > + else:
> > + cond = "trace_event_get_state(%s)" % event_id
> > + out(' if (%(cond)s) {',
> > + ' _simple_%(api)s(%(args)s);',
> > + ' }',
> > api=event.api(),
> > + cond=cond,
> > args=", ".join(event.args.names()))
> >
> >
> > @@ -72,22 +81,10 @@ def generate_c(event, group):
> > if len(event.args) == 0:
> > sizestr = '0'
> >
> > - event_id = 'TRACE_' + event.name.upper()
> > - if "vcpu" in event.properties:
> > - # already checked on the generic format code
> > - cond = "true"
> > - else:
> > - cond = "trace_event_get_state(%s)" % event_id
> > -
> > out('',
> > - ' if (!%(cond)s) {',
> > - ' return;',
> > - ' }',
> > - '',
> > ' if (trace_record_start(&rec, %(event_obj)s.id,
> %(size_str)s)) {',
> > ' return; /* Trace Buffer Full, Event Dropped ! */',
> > ' }',
> > - cond=cond,
> > event_obj=event.api(event.QEMU_EVENT),
> > size_str=sizestr)
> >
> > --
> > 2.34.1
> >
> >
>
>
