Yes, you're right—there was a mistake in the original commit description.
In this change, we're moving cold paths out of the header (.h) files and
into corresponding source (.c) files. The primary motivation is to avoid
replicating the cold path logic across every translation unit that includes
trace.h (which includes trace-*.h headers).
For the syslog backend, which involves only a single function call, this
change won't have a noticeable impact. However, for the ftrace and log
backends, where functions like _ftrace_trace_foo() and _log_trace_foo() contain
more complex logic, this avoids significant duplication. By centralizing
the implementation in .c files, we expect to save memory and reduce binary
size.
Paolo and I are still experimenting to quantify these memory savings. So
far, we haven’t observed substantial gains, but we're refining our
measurements and isolating symbols more carefully to get accurate binary
size comparisons.
On Tue, Jun 3, 2025 at 3:56 AM Stefan Hajnoczi <stefa...@redhat.com> wrote:
> On Sun, Jun 01, 2025 at 06:12:29PM +0000, Tanish Desai wrote:
> > inline: move hot paths from .c to .h for better performance
> > Moved frequently used hot paths from the .c file to the .h file to
> enable inlining
> > and improve performance. This approach is inspired by past QEMU
> optimizations,
> > where performance-critical code was inlined based on profiling results.
> >
> > Signed-off-by: Tanish Desai <tanishdesa...@gmail.com>
> > ---
> > scripts/tracetool/backend/syslog.py | 36 +++++++++++++++++++++++------
> > 1 file changed, 29 insertions(+), 7 deletions(-)
>
> This commit description doesn't match what the patch does.
>
> What is the purpose of creating a _syslog_trace_foo() function in the .c
> file instead of calling syslog() directly from the .h file?
>
> >
> > diff --git a/scripts/tracetool/backend/syslog.py
> b/scripts/tracetool/backend/syslog.py
> > index 012970f6cc..52b8ff65ea 100644
> > --- a/scripts/tracetool/backend/syslog.py
> > +++ b/scripts/tracetool/backend/syslog.py
> > @@ -21,8 +21,12 @@
> >
> >
> > def generate_h_begin(events, group):
> > - out('#include <syslog.h>',
> > - '')
> > + out('#include <syslog.h>')
> > + for event in events:
> > + out('void _syslog_%(api)s(%(args)s);',
> > + api=event.api(),
> > + args=event.args)
> > + out('')
> >
> >
> > def generate_h(event, group):
> > @@ -37,17 +41,35 @@ def generate_h(event, group):
> > cond = "trace_event_get_state(%s)" % ("TRACE_" +
> event.name.upper())
> >
> > out(' if (%(cond)s) {',
> > - '#line %(event_lineno)d "%(event_filename)s"',
> > - ' syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
> > - '#line %(out_next_lineno)d "%(out_filename)s"',
> > + ' _syslog_%(api)s(%(args)s);',
> > ' }',
> > cond=cond,
> > event_lineno=event.lineno,
> > event_filename=os.path.relpath(event.filename),
> > name=event.name,
> > fmt=event.fmt.rstrip("\n"),
> > - argnames=argnames)
> > -
> > + argnames=argnames,
> > + api=event.api(),
> > + args=", ".join(event.args.names()))
> > +
> > +
> > +def generate_c(event, group):
> > + argnames = ", ".join(event.args.names())
> > + if len(event.args) > 0:
> > + argnames = ", " + argnames
> > + out('void _syslog_%(api)s(%(args)s){',
> > + ' #line %(event_lineno)d "%(event_filename)s"',
> > + ' syslog(LOG_INFO, "%(name)s " %(fmt)s
> %(argnames)s);',
> > + ' #line %(out_next_lineno)d "%(out_filename)s"',
> > + '}',
> > + '',
> > + event_lineno=event.lineno,
> > + event_filename=os.path.relpath(event.filename),
> > + name=event.name,
> > + fmt=event.fmt.rstrip("\n"),
> > + argnames=argnames,
> > + api=event.api(),
> > + args=event.args)
> >
> > def generate_h_backend_dstate(event, group):
> > out(' trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
> > --
> > 2.34.1
> >
>
