Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 13/07/19 16:15, Markus Armbruster wrote:
>>>                    In particular the tracing headers are using
>>> $(build_root)/$(<D); for now my solution is to generate headers like
>>> "trace/trace-audio.h" and have sixty one-line forwarding headers in the
>>> source tree; for example "audio/trace.h" includes "trace/trace-audio.h".
>>> I'm not sure if it's possible to instead add a one-line "generate
>>> trace headers" directive to each subdirectory's meson.build file.
>>> I suspect that it _is_ possible but you'd still have to change the
>>> include directives to include the subdirectory name (and then I prefer
>>> the forwarding headers).
>> 
>> I agree we want to keep '#include "trace.h"'.

Hmm, maybe we don't.

Having to add '#include "trace.h"' to FOO.c along with the first
tracepoint isn't really valuable.  It's not terrible, either.

What we don't want is having to add '#include "trace-FOO.h", because
that crosses the annoyance line.

Can we avoid having to add any #include to FOO.c?

The include has to happen *somewhere*, of course.  Can we make it happen
in qemu/osdep.h?

Appending

    #include "trace.h"

to it works as long as there is a "trace.h", but also makes even .o not
using tracepoints depend on it.  No good.

So make it conditional:

    #ifdef USING_TRACEPOINTS
    #include "trace.h"
    #endif

with suitable provision to compile FOO.c with -DUSING_TRACEPOINTS
exactly when it uses tracepoints.

Ways to do that:

* Add 'FOO.o-cflags := -DUSING_TRACEPOINTS' to Makefile.objs.  This is
  even worse than having to add '#include "trace-FOO.h".

* Add something like 'trace-using-obj += FOO.c' to build up a list of
  tracepoint using .c, then use it to add the -D in one place.  This
  could be tolerable, I guess.

* Generate the list from trace-events.  Now we're talking.

At this point, we can just as well add

    #ifdef TRACE_HEADER
    #include TRACE_HEADER
    #endif

and pass -DTRACE_HEADER='"trace-DIR.h"'.  Look Ma, no forwarding
headers!

But avoiding the silly forwarding headers with Meson this is not even
half the advantage.  Let me take a short detour away from Meson.

We have trace-events with hundreds of tracepoints for tens of source
files.  The generated trace.h clock in at hundreds of KiB for me.
Messing with tracepoints in one source file recompiles most of the
directory.  This is so much better than it used to be, but clearly not
as good as it could be.

The worst of the lot is trace-root.h, which gets included for >1300 .o
in my "build everything" tree, mostly because it contains tracepoints
for static inline functions in widely included headers.  See also
"[PATCH 07/28] trace: Do not include qom/cpu.h into generated trace.h",
Message-Id: <address@hidden>.

We started with a single trace-events.  That wasn't good, so we split it
up into one per directory.  That isn't good, so what about splitting it
up into one per source file?  Pass -DTRACE_HEADER='"trace-DIR-FOO.h"
instead of -DTRACE_HEADER='"trace-DIR.h"' when compiling DIR/FOO.c.

This is admittedly a half-baked idea.  It doesn't address tracepoints in
headers, yet.  But those headers are rare.  A quick grep finds some
twenty possibles.  Perhaps we can tolerate manual #include there.

Since we're discussing half-baked ideas already, let me throw out
another one: move the tracepoint declarations from trace-events into the
source files.  No more silly merge conflicts between patches related
only through a shared trace-events file.

One more: make the format string optional, default to one containing the
declared parameter names and the obvious conversion specifications.
Because format strings like this one

    visit_start_struct(void *v, const char *name, void *obj, size_t size) "v=%p 
name=%s obj=%p size=%zu"

add zero bits of information to the declaration preceding it :)

[...]