[Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Markus Armbruster]

This is kind of a meta-cover-letter for the multiple include cleanup
series I hope to post as time permits.  The first one will go out later
today.

I'm afraid it's a bit long.  You may want to skip ahead to "= What to do
about it =".

Stefan cc'ed because tracing is part of the problem.  Search for
"tracers".


= The status quo and why I hate it =

I've seen several schools of thought on use of #include.

There's the "no #include in headers" school: every .c file includes
exactly the headers it needs, and the prerequisites they need.  Cyclic
inclusion becomes impossible.  You can't sweep cyclic dependencies under
the rug.  Headers are read just once per compilation unit.  The amount
of crap you include is clearly visible.  However, maintaining the
#include directives is a drag, not least because their order matters.
Especially when headers neglect to spell out their dependencies.  Or
they do, but it's wrong.

There's the "headers must be self-contained" school: every header
includes everything it needs.  Headers can be included in any order.
Sorted #include directives are tidy and easy to navigate.  Headers can
be read multiple times, which can only hurt compilation time.  You need
to make an effort to avoid cyclic dependencies and excessive inclusion.

And then there's the school of non-thought: when it doesn't compile,
sprinkle #include on the mess semi-randomly until it does.

We do a bit of all three, but the result looks awfully close to what the
school of non-thought produces.

Every .c file includes qemu/osdep.h first.  For me, a .c file that
includes nothing but that comes out well over half a Megabyte in >23k
lines preprocessed.  Where does all this crap come from?

  #lines  KiBytes  #files  source
    5233     102       5   QEMU
    8035     159      70   system
    7915     224      73   GLib
    2458      89       1   # lines
   23641     576     149   total

"# lines" are lines added by the preprocessor so the rest of the
compiler can keep track of source locations.

Having the compiler wade through almost half a Megabyte of system+GLib
crap before it begins to consider the code we care about feels wasteful.
Perhaps we should rethink our approach to including library headers.

Of the 102K that are actually our own, just 7K come from include/.  95K
come from qapi-types.h.

Judging from the .d files in my build tree, 95% of the .c files include
qemu-common.h.  That makes things a good deal worse.  Without
NEED_CPU_H, this adds a modest 44K of our own headers, but almost 100K
of system headers:

  #lines  KiBytes  #files  source
    6938     146      16   QEMU
   11426     254      74   system
    7915     224      73   GLib
    2658     100       1   # lines
   28937     726     164   total

NEED_CPU_H adds another 120K of our own headers:

  #lines  KiBytes  #files  source
   11534     263      43   QEMU
   11548     256      78   system
    7915     225      72   GLib
    3370     138       1   # lines
   34367     883     194   total

The average size of a .c file is just over 15KiB.  To get to the actual
C code there, the compiler has to wade through at least 550-880KiB of
headers.  In other words, roughly 2% of the source comes from .c in the
best case.

But that's not even the worst part.  The worst part by far are our
"touch this and recompile the world" headers.

I find just short 4000 .d files in my build tree.  Guess how many of our
headers are listed as prerequisites in more than 90% of them (thus
touching them will recompile the .c file)?  *Twenty-two*.  Almost fifty
recompile more half of the world.

Naturally, touching osdep.h or anything it includes recompiles the
world.  These are:

    config-host.h
    include/glib-compat.h
    include/qapi/error.h
    include/qemu/compiler.h
    include/qemu/osdep.h
    include/qemu/typedefs.h
    include/sysemu/os-posix.h
    qapi-types.h

NEED_CPU_H adds

    config-target.h

Fine, except for qapi/error.h and qapi-types.h.  The latter is an itch I
need to scratch urgently.  My first patch series will take a swing at
it.

qemu-common.h without NEED_CPU_H adds

    include/fpu/softfloat.h
    include/qapi/qmp/qdict.h
    include/qapi/qmp/qlist.h
    include/qapi/qmp/qobject.h
    include/qemu-common.h
    include/qemu/atomic.h
    include/qemu/bswap.h
    include/qemu/fprintf-fn.h
    include/qemu/host-utils.h
    include/qemu/module.h
    include/qemu/option.h
    include/qemu/queue.h

Remember, these get included into 95% of all .c files.  I'm pretty sure
the fraction that actually needs QDicts or QemuOpts is much, much lower.

NEED_CPU_H further adds

    include/disas/bfd.h
    include/exec/cpu-all.h
    include/exec/cpu-common.h
    include/exec/cpu-defs.h
    include/exec/exec-all.h
    include/exec/hwaddr.h
    include/exec/memattrs.h
    include/exec/memory.h
    include/hw/hotplug.h
    include/hw/i386/apic.h
    include/hw/irq.h
    include/hw/qdev-core.h
    include/qemu/bitmap.h
    include/qemu/bitops.h
    include/qemu/int128.h
    include/qemu/log.h
    include/qemu/notify.h
    include/qemu/rcu.h
    include/qemu/thread-posix.h
    include/qemu/thread.h
    include/qom/cpu.h
    include/qom/object.h
    include/standard-headers/asm-x86/hyperv.h
    include/standard-headers/linux/types.h
    target-i386/cpu-qom.h
    target-i386/cpu.h
    target-i386/svm.h
    tcg/i386/tcg-target.h
    x86_64-softmmu/config-target.h

A fun exercise is to count occurences of each header in .d files and
multiply their number by their size.  That's the number of bytes read
from them when compiling from scratch.  Top scorers:

 size * count    size   count
    525760413  698221     753 trace/generated-tracers.h
    298039140   93723    3180 qapi-types.h
    197442619   55759    3541 include/qom/object.h
    185845916   53884    3449 include/exec/memory.h
    143750444   36878    3898 /usr/include/glib-2.0/glib/gunicode.h
    117362690   30643    3830 include/fpu/softfloat.h
    109783272   28164    3898 /usr/include/glib-2.0/glib/gregex.h
    105830700   27150    3898 /usr/include/glib-2.0/glib/gvariant.h
     92972157  123469     753 trace/generated-events.h
     88706786   22757    3898 /usr/include/glib-2.0/glib/gtestutils.h

The grand total of size * count is 5.4 GiBytes.  That's ~4600 times the
size of all .c and .h files in the repo :)


= What to do about it =

The immediately obvious thing to do is reduce "recompile the world"
headers that change frequently.  I've started to do that.

Another one is attacking widely included bulky files (see "Top
scorers").  Some can simply be included less.  Others need to be split,
in particular the generated tracers.

Yet another one is reviewing the way we include system and GLib headers.

But our root problem is our undisciplined use of #include.  Can we agree
on a sane set of rules?  Here's my proposal:

1. Have a carefully curated header that's included everywhere first.  We
   got that already thanks to Peter: osdep.h.

2. Headers should normally include everything they need beyond osdep.h.
   If exceptions are needed for some reason, they must be documented in
   the header.  If all that's needed from a header is typedefs, put
   those into qemu/typedefs.h instead of including the header.

3. Cyclic inclusion is forbidden.

Nice to have: "make check" checks 2. and 3.

Opinions?

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Paolo Bonzini]

On 15/03/2016 10:29, Markus Armbruster wrote:
> NEED_CPU_H further adds
> 
>     include/disas/bfd.h
>     include/exec/cpu-all.h
>     include/exec/cpu-common.h
>     include/exec/cpu-defs.h
>     include/exec/exec-all.h
>     include/exec/hwaddr.h
>     include/exec/memattrs.h
>     include/exec/memory.h
>     include/hw/hotplug.h
>     include/hw/i386/apic.h
>     include/hw/irq.h
>     include/hw/qdev-core.h
>     include/qemu/bitmap.h
>     include/qemu/bitops.h
>     include/qemu/int128.h
>     include/qemu/log.h
>     include/qemu/notify.h
>     include/qemu/rcu.h
>     include/qemu/thread-posix.h
>     include/qemu/thread.h
>     include/qom/cpu.h
>     include/qom/object.h
>     include/standard-headers/asm-x86/hyperv.h
>     include/standard-headers/linux/types.h
>     target-i386/cpu-qom.h
>     target-i386/cpu.h
>     target-i386/svm.h
>     tcg/i386/tcg-target.h
>     x86_64-softmmu/config-target.h

FWIW, I'm going to do something about this in 2.7.  I have patches to
remove cpu.h inclusion from qemu-common.h.

Generally though qemu-common.h needs to go. :)
Paolo

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Peter Maydell]

On 15 March 2016 at 09:29, Markus Armbruster <armbru@redhat.com> wrote:
> = What to do about it =
>
> The immediately obvious thing to do is reduce "recompile the world"
> headers that change frequently.  I've started to do that.

This is obviously worthwhile.

> Another one is attacking widely included bulky files (see "Top
> scorers").  Some can simply be included less.  Others need to be split,
> in particular the generated tracers.

But how important is this? Yes, the headers may make up a large
%-of-total-source-lines, but how much extra actual compile time
do they add? My off-the-top-of-my-head guess is that most of the
time will be spent on optimization passes of actual code, and that
things like structure definitions, typedefs, etc that you find in
headers aren't going to make a major contribution.

> Yet another one is reviewing the way we include system and GLib headers.

We must include glib.h from osdep.h because it has compatibility
fixups (in glib-compat.h) and we need to avoid the risk of a .c
file including the system header directly without the fixups.

> But our root problem is our undisciplined use of #include.  Can we agree
> on a sane set of rules?  Here's my proposal:
>
> 1. Have a carefully curated header that's included everywhere first.  We
>    got that already thanks to Peter: osdep.h.
>
> 2. Headers should normally include everything they need beyond osdep.h.
>    If exceptions are needed for some reason, they must be documented in
>    the header.  If all that's needed from a header is typedefs, put
>    those into qemu/typedefs.h instead of including the header.
>
> 3. Cyclic inclusion is forbidden.

I would be happy with these rules; we're a fair way away from them
though. (Part of my aim with the osdep cleanup was to get to a point
where it was easier to make header files self-contained without that
implying every header needing a lot of system #includes.)

thanks
-- PMM

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/03/2016 10:29, Markus Armbruster wrote:
>> NEED_CPU_H further adds
>> 
>>     include/disas/bfd.h
>>     include/exec/cpu-all.h
>>     include/exec/cpu-common.h
>>     include/exec/cpu-defs.h
>>     include/exec/exec-all.h
>>     include/exec/hwaddr.h
>>     include/exec/memattrs.h
>>     include/exec/memory.h
>>     include/hw/hotplug.h
>>     include/hw/i386/apic.h
>>     include/hw/irq.h
>>     include/hw/qdev-core.h
>>     include/qemu/bitmap.h
>>     include/qemu/bitops.h
>>     include/qemu/int128.h
>>     include/qemu/log.h
>>     include/qemu/notify.h
>>     include/qemu/rcu.h
>>     include/qemu/thread-posix.h
>>     include/qemu/thread.h
>>     include/qom/cpu.h
>>     include/qom/object.h
>>     include/standard-headers/asm-x86/hyperv.h
>>     include/standard-headers/linux/types.h
>>     target-i386/cpu-qom.h
>>     target-i386/cpu.h
>>     target-i386/svm.h
>>     tcg/i386/tcg-target.h
>>     x86_64-softmmu/config-target.h
>
> FWIW, I'm going to do something about this in 2.7.  I have patches to
> remove cpu.h inclusion from qemu-common.h.

Appreciated!

> Generally though qemu-common.h needs to go. :)

Yes.  The series I hope to post today is a baby step in that direction.

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 15 March 2016 at 09:29, Markus Armbruster <armbru@redhat.com> wrote:
>> = What to do about it =
>>
>> The immediately obvious thing to do is reduce "recompile the world"
>> headers that change frequently.  I've started to do that.
>
> This is obviously worthwhile.
>
>> Another one is attacking widely included bulky files (see "Top
>> scorers").  Some can simply be included less.  Others need to be split,
>> in particular the generated tracers.
>
> But how important is this? Yes, the headers may make up a large
> %-of-total-source-lines, but how much extra actual compile time
> do they add? My off-the-top-of-my-head guess is that most of the
> time will be spent on optimization passes of actual code, and that
> things like structure definitions, typedefs, etc that you find in
> headers aren't going to make a major contribution.

I readily concede that we shouldn't speculate about compile time without
measurements.

We need to do something about the generated tracers, though.  Every time
you fiddle with a tracepoint, you get to recompile ~900 out of ~4000
files.  I suspect it's only 900 mostly because people avoid tracepoints
for that exact reason.  I certainly do.  If only the fiddled-with file
got recompiled, I'd happily use them for debugging, and when done post
any new tracepoints I found useful for others to enjoy.

>> Yet another one is reviewing the way we include system and GLib headers.
>
> We must include glib.h from osdep.h because it has compatibility
> fixups (in glib-compat.h) and we need to avoid the risk of a .c
> file including the system header directly without the fixups.

I understand the problem.  Perhaps we can find a less heavy-handed
solution.  Not a priority for me right now.

>> But our root problem is our undisciplined use of #include.  Can we agree
>> on a sane set of rules?  Here's my proposal:
>>
>> 1. Have a carefully curated header that's included everywhere first.  We
>>    got that already thanks to Peter: osdep.h.
>>
>> 2. Headers should normally include everything they need beyond osdep.h.
>>    If exceptions are needed for some reason, they must be documented in
>>    the header.  If all that's needed from a header is typedefs, put
>>    those into qemu/typedefs.h instead of including the header.
>>
>> 3. Cyclic inclusion is forbidden.
>
> I would be happy with these rules; we're a fair way away from them
> though. (Part of my aim with the osdep cleanup was to get to a point
> where it was easier to make header files self-contained without that
> implying every header needing a lot of system #includes.)

Okay, I can try to take us a few more steps in this direction.

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Peter Maydell]

On 15 March 2016 at 12:28, Markus Armbruster <armbru@redhat.com> wrote:
> We need to do something about the generated tracers, though.  Every time
> you fiddle with a tracepoint, you get to recompile ~900 out of ~4000
> files.  I suspect it's only 900 mostly because people avoid tracepoints
> for that exact reason.  I certainly do.  If only the fiddled-with file
> got recompiled, I'd happily use them for debugging, and when done post
> any new tracepoints I found useful for others to enjoy.

Yes, definitely -- having a single file listing every tracepoint
in the system is a recipe for merge conflicts too, so splitting
it would be sensible. (Either lots of separate files, or maybe we
could put tracepoint definitions in the .c files and have the
scripts pull them out...)

thanks
-- PMM

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

On Tue, Mar 15, 2016 at 10:29:06AM +0100, Markus Armbruster wrote:
> Stefan cc'ed because tracing is part of the problem.  Search for
> "tracers".

Thanks for bringing up the generated tracing headers.  David Gilbert and
I discussed splitting them up a few months ago.  I'll summarize the
problem and we can think about solutions:

trace.h includes generated-tracers.h and generated-events.h.  These
files are generated from scripts/tracetool.py.  The contents of the
files depends on the configured --enable-trace-backends= list (log,
simple, ftrace, dtrace, ust).

Idealy ./trace-events would have "sections" so that multiple files are
generated:

  # section virtio-blk
  foo() ...

  # section memory
  bar() ...

This would put trace_foo() in generated-tracers-virtio-blk.h and
trace_bar() in generated-tracers-memory.h.  Source files using tracing
would need to include headers for relevant sections.

This way we can narrow the scope of tracing headers and prevent global
recompilation.

The fly in the ointment is that trace/control.h defines enum
TraceEventID, a global numbering of all trace events.  The enum is used
in trace/contro.h APIs and also in the simpletrace file format.

If ./trace-event is modified the numbering of trace events could change.
This would require global recompilation :(.

So in order to avoid global recompilation we need to eliminate enum
TraceEventID.  Perhaps it's possible to use TraceEvent* instead of
TraceEventID.  For the simpletrace backend we would continue to use a
global ordering but that only affects generated-tracers.c and not header
files (thankfully!).

Any comments?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Daniel P. Berrange]

On Tue, Mar 15, 2016 at 01:39:17PM +0000, Stefan Hajnoczi wrote:
> On Tue, Mar 15, 2016 at 10:29:06AM +0100, Markus Armbruster wrote:
> > Stefan cc'ed because tracing is part of the problem.  Search for
> > "tracers".
> 
> Thanks for bringing up the generated tracing headers.  David Gilbert and
> I discussed splitting them up a few months ago.  I'll summarize the
> problem and we can think about solutions:
> 
> trace.h includes generated-tracers.h and generated-events.h.  These
> files are generated from scripts/tracetool.py.  The contents of the
> files depends on the configured --enable-trace-backends= list (log,
> simple, ftrace, dtrace, ust).
> 
> Idealy ./trace-events would have "sections" so that multiple files are
> generated:
> 
>   # section virtio-blk
>   foo() ...
> 
>   # section memory
>   bar() ...
> 
> This would put trace_foo() in generated-tracers-virtio-blk.h and
> trace_bar() in generated-tracers-memory.h.  Source files using tracing
> would need to include headers for relevant sections.

I'd actually like to see the trace events associated with the subsystem
eg any trace events for block devices should live in block/trace-events,
likewise for things like crypto/trace-events, io/trace-events, etc, etc.

This would naturally lead to generating a block/trace-events.h file that
only contained the block trace point definitions.

As well as simplifying compilation deps, it means the trace-events file
changes get associated with correct people for get_maintainer.pl usage,
and we reduce liklihood of merge conflicts for pull requests by getting
rid of the single big file

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> On Tue, Mar 15, 2016 at 10:29:06AM +0100, Markus Armbruster wrote:
> > Stefan cc'ed because tracing is part of the problem.  Search for
> > "tracers".
> 
> Thanks for bringing up the generated tracing headers.  David Gilbert and
> I discussed splitting them up a few months ago.  I'll summarize the
> problem and we can think about solutions:
> 
> trace.h includes generated-tracers.h and generated-events.h.  These
> files are generated from scripts/tracetool.py.  The contents of the
> files depends on the configured --enable-trace-backends= list (log,
> simple, ftrace, dtrace, ust).
> 
> Idealy ./trace-events would have "sections" so that multiple files are
> generated:
> 
>   # section virtio-blk
>   foo() ...
> 
>   # section memory
>   bar() ...

I was going to split that the other way, and have
  trace-events
  migration/trace-events
  block/trace-events

and then they each generate their own header.

> This would put trace_foo() in generated-tracers-virtio-blk.h and
> trace_bar() in generated-tracers-memory.h.  Source files using tracing
> would need to include headers for relevant sections.
> 
> This way we can narrow the scope of tracing headers and prevent global
> recompilation.
> 
> The fly in the ointment is that trace/control.h defines enum
> TraceEventID, a global numbering of all trace events.  The enum is used
> in trace/contro.h APIs and also in the simpletrace file format.
> 
> If ./trace-event is modified the numbering of trace events could change.
> This would require global recompilation :(.
> 
> So in order to avoid global recompilation we need to eliminate enum
> TraceEventID.  Perhaps it's possible to use TraceEvent* instead of
> TraceEventID.  For the simpletrace backend we would continue to use a
> global ordering but that only affects generated-tracers.c and not header
> files (thankfully!).

I think the hardest part is going to be understanding all of the different
tracers and the things they need to know; like that thing about simpletrace
needing the single ordering;  I don't know what the other backends need
but I bet they've each got their own surprise.

(And yes, I'd love to split them up - I tend to use traceing quite a bit
in migration now and trying to do a bisect over a big migration patch
series takes ages mostly because of the trace.h changes)

Dave

> 
> Any comments?
> 
> Stefan


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Paolo Bonzini]

On 15/03/2016 13:19, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 15/03/2016 10:29, Markus Armbruster wrote:
>>> NEED_CPU_H further adds
>>>
>>>     include/disas/bfd.h
>>>     include/exec/cpu-all.h
>>>     include/exec/cpu-common.h
>>>     include/exec/cpu-defs.h
>>>     include/exec/exec-all.h
>>>     include/exec/hwaddr.h
>>>     include/exec/memattrs.h
>>>     include/exec/memory.h
>>>     include/hw/hotplug.h
>>>     include/hw/i386/apic.h
>>>     include/hw/irq.h
>>>     include/hw/qdev-core.h
>>>     include/qemu/bitmap.h
>>>     include/qemu/bitops.h
>>>     include/qemu/int128.h
>>>     include/qemu/log.h
>>>     include/qemu/notify.h
>>>     include/qemu/rcu.h
>>>     include/qemu/thread-posix.h
>>>     include/qemu/thread.h
>>>     include/qom/cpu.h
>>>     include/qom/object.h
>>>     include/standard-headers/asm-x86/hyperv.h
>>>     include/standard-headers/linux/types.h
>>>     target-i386/cpu-qom.h
>>>     target-i386/cpu.h
>>>     target-i386/svm.h
>>>     tcg/i386/tcg-target.h
>>>     x86_64-softmmu/config-target.h
>>
>> FWIW, I'm going to do something about this in 2.7.  I have patches to
>> remove cpu.h inclusion from qemu-common.h.
> 
> Appreciated!

Now pushed to github.com/bonzini/qemu.git branch need-cpu-h.
Shortlog follows:

Paolo Bonzini (48):
      include: move some definitions out of qemu-common.h
      log: do not use CONFIG_USER_ONLY
      hw: explicitly include qemu-common.h and cpu.h
      cpu: make cpu-qom.h only include-able from cpu.h
      target-alpha: make cpu-qom.h not target specific
      target-arm: make cpu-qom.h not target specific
      target-cris: make cpu-qom.h not target specific
      target-i386: make cpu-qom.h not target specific
      target-lm32: make cpu-qom.h not target specific
      target-m68k: make cpu-qom.h not target specific
      target-microblaze: make cpu-qom.h not target specific
      target-mips: make cpu-qom.h not target specific
      target-ppc: do not use target_ulong in cpu-qom.h
      target-ppc: make cpu-qom.h not target specific
      target-s390x: make cpu-qom.h not target specific
      target-sh4: make cpu-qom.h not target specific
      target-sparc: make cpu-qom.h not target specific
      target-tricore: make cpu-qom.h not target specific
      target-unicore32: make cpu-qom.h not target specific
      target-xtensa: make cpu-qom.h not target specific
      arm: include cpu-qom.h in files that require ARMCPU
      m68k: include cpu-qom.h in files that require M68KCPU
      sh4: include cpu-qom.h in files that require M68KCPU
      alpha: include cpu-qom.h in files that require AlphaCPU
      mips: use MIPSCPU instead of CPUMIPSState
      ppc: use PowerPCCPU instead of CPUPPCState
      arm: remove useless cpu.h inclusion
      explicitly include qom/cpu.h
      explicitly include hw/qdev-core.h
      explicitly include linux/kvm.h
      apic: move target-dependent definitions to cpu.h
      include: poison symbols in osdep.h
      hw: do not use VMSTATE_*TL
      hw: move CPU state serialization to migration/cpu.h
      hw: cannot include hw/hw.h from user emulation
      cpu: move endian-dependent load/store functions to cpu-all.h
      qemu-common: stop including qemu/bswap.h from qemu-common.h
      qemu-common: stop including qemu/host-utils.h from qemu-common.h
      gdbstub: remove includes from gdbstub-xml.c
      dma: do not depend on kvm_enabled()
      do not include qemu-common.h in hw/hw.h
      s390x: move stuff out of cpu.h
      qemu-common: push cpu.h inclusion out of qemu-common.h
      arm: move arm_log_exception into .c file
      mips: move CP0 functions out of cpu.h
      hw: explicitly include qemu/log.h
      exec: extract exec/tb-context.h
      cpu: move exec-all.h inclusion out of cpu.h

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2110 bytes --]

On Tue, Mar 15, 2016 at 01:56:47PM +0000, Dr. David Alan Gilbert wrote:
> > This would put trace_foo() in generated-tracers-virtio-blk.h and
> > trace_bar() in generated-tracers-memory.h.  Source files using tracing
> > would need to include headers for relevant sections.
> > 
> > This way we can narrow the scope of tracing headers and prevent global
> > recompilation.
> > 
> > The fly in the ointment is that trace/control.h defines enum
> > TraceEventID, a global numbering of all trace events.  The enum is used
> > in trace/contro.h APIs and also in the simpletrace file format.
> > 
> > If ./trace-event is modified the numbering of trace events could change.
> > This would require global recompilation :(.
> > 
> > So in order to avoid global recompilation we need to eliminate enum
> > TraceEventID.  Perhaps it's possible to use TraceEvent* instead of
> > TraceEventID.  For the simpletrace backend we would continue to use a
> > global ordering but that only affects generated-tracers.c and not header
> > files (thankfully!).
> 
> I think the hardest part is going to be understanding all of the different
> tracers and the things they need to know; like that thing about simpletrace
> needing the single ordering;  I don't know what the other backends need
> but I bet they've each got their own surprise.
> 
> (And yes, I'd love to split them up - I tend to use traceing quite a bit
> in migration now and trying to do a bisect over a big migration patch
> series takes ages mostly because of the trace.h changes)

Yes, this is the hard part.  The simpletrace.py pretty-printing script
takes a trace-events file as input.  I suppose it could alphabetically
sort the trace-events filenames from the command-line to produce a
global ordering of trace event IDs.  Of course that doesn't work
properly if the script is invoked with relative paths from a
sub-directory...

Perhaps we need a new script during the QEMU build process that produces
a trace-event-ids.csv file.  Then simpletrace.py could take that instead
of processing ./trace-events.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> On Tue, Mar 15, 2016 at 01:56:47PM +0000, Dr. David Alan Gilbert wrote:
> > > This would put trace_foo() in generated-tracers-virtio-blk.h and
> > > trace_bar() in generated-tracers-memory.h.  Source files using tracing
> > > would need to include headers for relevant sections.
> > > 
> > > This way we can narrow the scope of tracing headers and prevent global
> > > recompilation.
> > > 
> > > The fly in the ointment is that trace/control.h defines enum
> > > TraceEventID, a global numbering of all trace events.  The enum is used
> > > in trace/contro.h APIs and also in the simpletrace file format.
> > > 
> > > If ./trace-event is modified the numbering of trace events could change.
> > > This would require global recompilation :(.
> > > 
> > > So in order to avoid global recompilation we need to eliminate enum
> > > TraceEventID.  Perhaps it's possible to use TraceEvent* instead of
> > > TraceEventID.  For the simpletrace backend we would continue to use a
> > > global ordering but that only affects generated-tracers.c and not header
> > > files (thankfully!).
> > 
> > I think the hardest part is going to be understanding all of the different
> > tracers and the things they need to know; like that thing about simpletrace
> > needing the single ordering;  I don't know what the other backends need
> > but I bet they've each got their own surprise.
> > 
> > (And yes, I'd love to split them up - I tend to use traceing quite a bit
> > in migration now and trying to do a bisect over a big migration patch
> > series takes ages mostly because of the trace.h changes)
> 
> Yes, this is the hard part.  The simpletrace.py pretty-printing script
> takes a trace-events file as input.  I suppose it could alphabetically
> sort the trace-events filenames from the command-line to produce a
> global ordering of trace event IDs.  Of course that doesn't work
> properly if the script is invoked with relative paths from a
> sub-directory...
> 
> Perhaps we need a new script during the QEMU build process that produces
> a trace-event-ids.csv file.  Then simpletrace.py could take that instead
> of processing ./trace-events.

Well that sounds easy - but is it easy to avoid using those fixed IDs
in any of the trace.h functions that are included everywhere (without
making them more expensive).

Dave

> Stefan


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2850 bytes --]

On Wed, Mar 16, 2016 at 06:27:48PM +0000, Dr. David Alan Gilbert wrote:
> * Stefan Hajnoczi (stefanha@redhat.com) wrote:
> > On Tue, Mar 15, 2016 at 01:56:47PM +0000, Dr. David Alan Gilbert wrote:
> > > > This would put trace_foo() in generated-tracers-virtio-blk.h and
> > > > trace_bar() in generated-tracers-memory.h.  Source files using tracing
> > > > would need to include headers for relevant sections.
> > > > 
> > > > This way we can narrow the scope of tracing headers and prevent global
> > > > recompilation.
> > > > 
> > > > The fly in the ointment is that trace/control.h defines enum
> > > > TraceEventID, a global numbering of all trace events.  The enum is used
> > > > in trace/contro.h APIs and also in the simpletrace file format.
> > > > 
> > > > If ./trace-event is modified the numbering of trace events could change.
> > > > This would require global recompilation :(.
> > > > 
> > > > So in order to avoid global recompilation we need to eliminate enum
> > > > TraceEventID.  Perhaps it's possible to use TraceEvent* instead of
> > > > TraceEventID.  For the simpletrace backend we would continue to use a
> > > > global ordering but that only affects generated-tracers.c and not header
> > > > files (thankfully!).
> > > 
> > > I think the hardest part is going to be understanding all of the different
> > > tracers and the things they need to know; like that thing about simpletrace
> > > needing the single ordering;  I don't know what the other backends need
> > > but I bet they've each got their own surprise.
> > > 
> > > (And yes, I'd love to split them up - I tend to use traceing quite a bit
> > > in migration now and trying to do a bisect over a big migration patch
> > > series takes ages mostly because of the trace.h changes)
> > 
> > Yes, this is the hard part.  The simpletrace.py pretty-printing script
> > takes a trace-events file as input.  I suppose it could alphabetically
> > sort the trace-events filenames from the command-line to produce a
> > global ordering of trace event IDs.  Of course that doesn't work
> > properly if the script is invoked with relative paths from a
> > sub-directory...
> > 
> > Perhaps we need a new script during the QEMU build process that produces
> > a trace-event-ids.csv file.  Then simpletrace.py could take that instead
> > of processing ./trace-events.
> 
> Well that sounds easy - but is it easy to avoid using those fixed IDs
> in any of the trace.h functions that are included everywhere (without
> making them more expensive).

How about this:

Instead of using TraceEventID in tracing APIs, pass the TraceEvent
pointer directly.  The tracetool.py code generator should produce
"extern TraceEvent trace_event_foo;" definitions in headers so that the
users have direct access to the TraceEvents.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> On Wed, Mar 16, 2016 at 06:27:48PM +0000, Dr. David Alan Gilbert wrote:
> > * Stefan Hajnoczi (stefanha@redhat.com) wrote:
> > > On Tue, Mar 15, 2016 at 01:56:47PM +0000, Dr. David Alan Gilbert wrote:
> > > > > This would put trace_foo() in generated-tracers-virtio-blk.h and
> > > > > trace_bar() in generated-tracers-memory.h.  Source files using tracing
> > > > > would need to include headers for relevant sections.
> > > > > 
> > > > > This way we can narrow the scope of tracing headers and prevent global
> > > > > recompilation.
> > > > > 
> > > > > The fly in the ointment is that trace/control.h defines enum
> > > > > TraceEventID, a global numbering of all trace events.  The enum is used
> > > > > in trace/contro.h APIs and also in the simpletrace file format.
> > > > > 
> > > > > If ./trace-event is modified the numbering of trace events could change.
> > > > > This would require global recompilation :(.
> > > > > 
> > > > > So in order to avoid global recompilation we need to eliminate enum
> > > > > TraceEventID.  Perhaps it's possible to use TraceEvent* instead of
> > > > > TraceEventID.  For the simpletrace backend we would continue to use a
> > > > > global ordering but that only affects generated-tracers.c and not header
> > > > > files (thankfully!).
> > > > 
> > > > I think the hardest part is going to be understanding all of the different
> > > > tracers and the things they need to know; like that thing about simpletrace
> > > > needing the single ordering;  I don't know what the other backends need
> > > > but I bet they've each got their own surprise.
> > > > 
> > > > (And yes, I'd love to split them up - I tend to use traceing quite a bit
> > > > in migration now and trying to do a bisect over a big migration patch
> > > > series takes ages mostly because of the trace.h changes)
> > > 
> > > Yes, this is the hard part.  The simpletrace.py pretty-printing script
> > > takes a trace-events file as input.  I suppose it could alphabetically
> > > sort the trace-events filenames from the command-line to produce a
> > > global ordering of trace event IDs.  Of course that doesn't work
> > > properly if the script is invoked with relative paths from a
> > > sub-directory...
> > > 
> > > Perhaps we need a new script during the QEMU build process that produces
> > > a trace-event-ids.csv file.  Then simpletrace.py could take that instead
> > > of processing ./trace-events.
> > 
> > Well that sounds easy - but is it easy to avoid using those fixed IDs
> > in any of the trace.h functions that are included everywhere (without
> > making them more expensive).
> 
> How about this:
> 
> Instead of using TraceEventID in tracing APIs, pass the TraceEvent
> pointer directly.  The tracetool.py code generator should produce
> "extern TraceEvent trace_event_foo;" definitions in headers so that the
> users have direct access to the TraceEvents.

OK, so I see TraceEvent has a TraceEventID field; so yes that works easily;
it turns out to be a little more expensive though since what was a:

   trace_events_dstate[id]

is now
   trace_events_dstate[te->id]

But hang on, what's the 'sstate' in TraceEvent; do we actually need two
state fields if we're passing a TraceEvent pointer around?

Dave

> Stefan


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Paolo Bonzini]

On 17/03/2016 17:29, Dr. David Alan Gilbert wrote:
> OK, so I see TraceEvent has a TraceEventID field; so yes that works easily;
> it turns out to be a little more expensive though since what was a:
> 
>    trace_events_dstate[id]
> 
> is now
>    trace_events_dstate[te->id]

That however makes you waste a lot of cache on trace_events_dstate
(commit 585ec72, "trace: track enabled events in a separate array",
2016-02-03).

Perhaps we get the linker to do compute the id, for example by using a
separate data section and then use te-&te_first to compute the id...
Richard, do you have ideas on how to do this in a reasonably portable
manner?

> But hang on, what's the 'sstate' in TraceEvent; do we actually need two
> state fields if we're passing a TraceEvent pointer around?

sstate means the event is unavailable, it's basically just a way to
provide better error messages.

Paolo

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Richard Henderson]

On 03/15/2016 02:29 AM, Markus Armbruster wrote:
> Headers can
> be read multiple times, which can only hurt compilation time.  You need
> to make an effort to avoid cyclic dependencies and excessive inclusion.

I just want to point out that the preprocessor understands the

#ifndef FOO
#define FOO
...
#endif

idiom, where nothing but comments precedes the #ifndef in the file, and
likewise nothing succeeds the matching #endif.  If FOO is still defined at the
next #include of the file, the preprocessor will skip the #include entirely.

So don't let this be a consideration in the argument.


r~

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 17/03/2016 17:29, Dr. David Alan Gilbert wrote:
> > OK, so I see TraceEvent has a TraceEventID field; so yes that works easily;
> > it turns out to be a little more expensive though since what was a:
> > 
> >    trace_events_dstate[id]
> > 
> > is now
> >    trace_events_dstate[te->id]
> 
> That however makes you waste a lot of cache on trace_events_dstate
> (commit 585ec72, "trace: track enabled events in a separate array",
> 2016-02-03).
> 
> Perhaps we get the linker to do compute the id, for example by using a
> separate data section and then use te-&te_first to compute the id...
> Richard, do you have ideas on how to do this in a reasonably portable
> manner?

It's possible we don't need the unique-id on the fast-path;  for example
if we had an id that was only unique within each trace-events file,
and it's own trace_events_dstate[] that used that ID, then that would still
be a nice statically known id.  You'd still need to be able to get the
global ID for the simple-event and anything else that needed it.

> > But hang on, what's the 'sstate' in TraceEvent; do we actually need two
> > state fields if we're passing a TraceEvent pointer around?
> 
> sstate means the event is unavailable, it's basically just a way to
> provide better error messages.

Dave

> 
> Paolo
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Paolo Bonzini]

On 17/03/2016 20:21, Paolo Bonzini wrote:
> On 17/03/2016 17:29, Dr. David Alan Gilbert wrote:
>> OK, so I see TraceEvent has a TraceEventID field; so yes that works easily;
>> it turns out to be a little more expensive though since what was a:
>>
>>    trace_events_dstate[id]
>>
>> is now
>>    trace_events_dstate[te->id]
> 
> That however makes you waste a lot of cache on trace_events_dstate
> (commit 585ec72, "trace: track enabled events in a separate array",
> 2016-02-03).
> 
> Perhaps we get the linker to do compute the id, for example by using a
> separate data section and then use te-&te_first to compute the id...
> Richard, do you have ideas on how to do this in a reasonably portable
> manner?

Dave and I actually got to a much better design on IRC.

1) since the .h files will be per-directory, make the dstate array per
directory (e.g. trace_events_dstate_migration[])

2) changes must have no effect on unrelated .h files, but we can keep a
global .c file.  Instead of using trace_events_dstate[te->id], we can
store a pointer directly in te, e.g. ".p_dstate =
&trace_events_dstate_migration[23]" and then *(te->p_dstate).  The
global .c file can also store mappings from local id to global id, so
that the global id can be retrieved with "trace_events_ids_migration[23]".

Processing trace-events would remain a global process, but unchanged .h
files would not be changed on disk and would not trigger the world.
Splitting the input to tracetool becomes a separate task from splitting
its output, and could be achieved simply by introducing include directives.

Paolo

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Richard Henderson]

On 03/17/2016 12:21 PM, Paolo Bonzini wrote:
> That however makes you waste a lot of cache on trace_events_dstate
> (commit 585ec72, "trace: track enabled events in a separate array",
> 2016-02-03).

I must say I'm not really convinced by that patch, since I don't see that
there's much locality between the ID's that would be polled.

> Perhaps we get the linker to do compute the id, for example by using a
> separate data section and then use te-&te_first to compute the id...
> Richard, do you have ideas on how to do this in a reasonably portable
> manner?

That works for all of the hosts we care about (ELF and MachO).

All you need is "crt0"/"crtn" files to force to be first and last, placing
symbols in known sections.  Then emit all of the "id"s as symbols in those
sections.

E.g. for an event FOO, generate

bool trace_event_FOO_dstate
  __attribute__((section(".bss.trace_event_dstate")));

TraceEvent trace_event_FOO
  __attribute__((section(".bss.trace_event")));

For portability, you must put the section attribute on the extern declaration
as well, lest the compiler make assumptions about small variables living in
".sbss" or the like, and thus being addressable with gp-relative offsets.

See gcc's crtstuff.c for examples of how to write the crt files.
Or, if you value your sanity, don't.  ;-)


r~

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Peter Maydell]

On 17 March 2016 at 20:14, Richard Henderson <rth@twiddle.net> wrote:
> That works for all of the hosts we care about (ELF and MachO).

...Windows is something other than both of those, right?
Or does it use ELF these days?

thanks
-- PMM

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Richard Henderson]

On 03/17/2016 01:27 PM, Peter Maydell wrote:
> On 17 March 2016 at 20:14, Richard Henderson <rth@twiddle.net> wrote:
>> That works for all of the hosts we care about (ELF and MachO).
> 
> ...Windows is something other than both of those, right?
> Or does it use ELF these days?

PECOFF.  But it still supports named sections, so not a deal-breaker.
We just don't support old a.out systems (e.g. really old OpenBSD).


r~

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Paolo Bonzini]

On 17/03/2016 21:14, Richard Henderson wrote:
> On 03/17/2016 12:21 PM, Paolo Bonzini wrote:
>> That however makes you waste a lot of cache on trace_events_dstate
>> (commit 585ec72, "trace: track enabled events in a separate array",
>> 2016-02-03).
> 
> I must say I'm not really convinced by that patch, since I don't see that
> there's much locality between the ID's that would be polled.

There are usually just three-four files in hot paths; they could be
kvm-all.c, memory.c, hw/virtio/virtio.c and hw/block/virtio-blk.c for a
disk benchmark for example.  All tracepoints for a file are adjacent,
hence the trace_events_dstate portion that represents one file (assuming
that file has <=64 events) costs 1-2 cache lines.  Without the patch the
footprint of trace_events is 1 cache line for every 2-3 events since
sizeof(TraceEvent) == 24.

It's true that the patch before 585ec72 also helps removing overhead in
the case where all events are disabled (and that's the really common
case).  That obviously avoids consuming _any_ amount of cache on
disabled trace events.  However I believe that separate dstate arrays
are anyway helpful for David's plan to split the generated tracing
headers and avoid world rebuilds.  That's because the headers only need
the dstate arrays and not the big global arrays.

Paolo

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Paolo Bonzini]

On 17/03/2016 21:29, Richard Henderson wrote:
>> > ...Windows is something other than both of those, right?
>> > Or does it use ELF these days?
> PECOFF.  But it still supports named sections, so not a deal-breaker.
> We just don't support old a.out systems (e.g. really old OpenBSD).

FWIW Windows also supports sorted sections.  You could put the "first"
symbol in data.trace_events$AAA, the last symbol in
data.trace_events$ZZZ and the intermediate symbols in
data.trace_events$FOO; the linker would merge them into a single
data.trace_events section and you wouldn't have to ensure the correct
order of the object files on the command line.

It's actually a pretty cool feature.  It makes such linker tricks much
easier to do.  But it's not portable.

Paolo

Re: [Qemu-devel] Our use of #include is undisciplined, and what to do about it

[Markus Armbruster]

It's been three years, let's examine how things have evolved.

I'm using commit db3d11ee3f0, which is a bit behind current master, just
so I can apply my "[PATCH 0/4] Cleanups around qemu-common.h" cleanly.

Markus Armbruster <armbru@redhat.com> writes:

[...]
> = The status quo and why I hate it =
>
> I've seen several schools of thought on use of #include.
>
> There's the "no #include in headers" school: every .c file includes
> exactly the headers it needs, and the prerequisites they need.  Cyclic
> inclusion becomes impossible.  You can't sweep cyclic dependencies under
> the rug.  Headers are read just once per compilation unit.  The amount
> of crap you include is clearly visible.  However, maintaining the
> #include directives is a drag, not least because their order matters.
> Especially when headers neglect to spell out their dependencies.  Or
> they do, but it's wrong.
>
> There's the "headers must be self-contained" school: every header
> includes everything it needs.  Headers can be included in any order.
> Sorted #include directives are tidy and easy to navigate.  Headers can
> be read multiple times, which can only hurt compilation time.

Our compilers avoid this for headers with proper header guards.

>                                                                You need
> to make an effort to avoid cyclic dependencies and excessive inclusion.
>
> And then there's the school of non-thought: when it doesn't compile,
> sprinkle #include on the mess semi-randomly until it does.
>
> We do a bit of all three, but the result looks awfully close to what the
> school of non-thought produces.
>
> Every .c file includes qemu/osdep.h first.  For me, a .c file that
> includes nothing but that comes out well over half a Megabyte in >23k
> lines preprocessed.  Where does all this crap come from?
>
>   #lines  KiBytes  #files  source
>     5233     102       5   QEMU
>     8035     159      70   system
>     7915     224      73   GLib
>     2458      89       1   # lines
>    23641     576     149   total
>
> "# lines" are lines added by the preprocessor so the rest of the
> compiler can keep track of source locations.

    #lines  KiBytes  #files  source
       375       8       5   QEMU
      9722     230     113   system
      8212     254      74   GLib
      1517      65     N/A   # lines
     19826     557     192   total

The weight QEMU lost, system + GLib put on.

> Having the compiler wade through almost half a Megabyte of system+GLib
> crap before it begins to consider the code we care about feels wasteful.
> Perhaps we should rethink our approach to including library headers.

No change.

> Of the 102K that are actually our own, just 7K come from include/.  95K
> come from qapi-types.h.

Fixed.

> Judging from the .d files in my build tree, 95% of the .c files include
> qemu-common.h.  That makes things a good deal worse.

Down to 90%.  My "[PATCH 0/4] Cleanups around qemu-common.h" shrinks it
to less than 10%.  Small enough for me not to repeat the measurements
below.

>                                                       Without
> NEED_CPU_H, this adds a modest 44K of our own headers, but almost 100K
> of system headers:
>
>   #lines  KiBytes  #files  source
>     6938     146      16   QEMU
>    11426     254      74   system
>     7915     224      73   GLib
>     2658     100       1   # lines
>    28937     726     164   total
>
> NEED_CPU_H adds another 120K of our own headers:
>
>   #lines  KiBytes  #files  source
>    11534     263      43   QEMU
>    11548     256      78   system
>     7915     225      72   GLib
>     3370     138       1   # lines
>    34367     883     194   total
>
> The average size of a .c file is just over 15KiB.  To get to the actual
> C code there, the compiler has to wade through at least 550-880KiB of
> headers.  In other words, roughly 2% of the source comes from .c in the
> best case.
>
> But that's not even the worst part.  The worst part by far are our
> "touch this and recompile the world" headers.
>
> I find just short 4000 .d files in my build tree.

Some 6400 now, ignoring the .d that don't contain ".o:".

>                                                    Guess how many of our
> headers are listed as prerequisites in more than 90% of them (thus
> touching them will recompile the .c file)?  *Twenty-two*.

Down to 12 before my "[PATCH 0/4] Cleanups around qemu-common.h", and to
10 afterwards.

>                                                            Almost fifty
> recompile more half of the world.

No significant change.

> Naturally, touching osdep.h or anything it includes recompiles the
> world.  These are:
>
>     config-host.h
>     include/glib-compat.h
>     include/qapi/error.h
>     include/qemu/compiler.h
>     include/qemu/osdep.h
>     include/qemu/typedefs.h
>     include/sysemu/os-posix.h
>     qapi-types.h
>
> NEED_CPU_H adds
>
>     config-target.h
>
> Fine, except for qapi/error.h and qapi-types.h.  The latter is an itch I
> need to scratch urgently.  My first patch series will take a swing at
> it.

Both are gone.

New: include/exec/poison.h

[...]
>
> A fun exercise is to count occurences of each header in .d files and
> multiply their number by their size.  That's the number of bytes read
> from them when compiling from scratch.  Top scorers:
>
>  size * count    size   count
>     525760413  698221     753 trace/generated-tracers.h
>     298039140   93723    3180 qapi-types.h
>     197442619   55759    3541 include/qom/object.h
>     185845916   53884    3449 include/exec/memory.h
>     143750444   36878    3898 /usr/include/glib-2.0/glib/gunicode.h
>     117362690   30643    3830 include/fpu/softfloat.h
>     109783272   28164    3898 /usr/include/glib-2.0/glib/gregex.h
>     105830700   27150    3898 /usr/include/glib-2.0/glib/gvariant.h
>      92972157  123469     753 trace/generated-events.h
>      88706786   22757    3898 /usr/include/glib-2.0/glib/gtestutils.h

After my "[PATCH 0/4] Cleanups around qemu-common.h":

   size * count    size   count
      428019130   82789    5170 include/exec/memory.h
      336965209   60857    5537 include/qom/object.h
      248105382   39121    6342 /usr/include/glib-2.0/glib/gunicode.h
      187247550   29525    6342 /usr/include/glib-2.0/glib/gvariant.h
      182359220  172037    1060 trace-root.h
      178178490   28095    6342 /usr/include/glib-2.0/glib/gregex.h
      167477688   71848    2331 qapi/qapi-types-block-core.h
      166926546   36947    4518 include/qom/cpu.h
      161105826   25403    6342 /usr/include/glib-2.0/glib/gmessages.h
      153508110   24205    6342 /usr/include/glib-2.0/glib/gtestutils.h

[...]
>
> = What to do about it =
>
> The immediately obvious thing to do is reduce "recompile the world"
> headers that change frequently.  I've started to do that.
>
> Another one is attacking widely included bulky files (see "Top
> scorers").  Some can simply be included less.  Others need to be split,
> in particular the generated tracers.

Some progress, notably around trace and QAPI.

> Yet another one is reviewing the way we include system and GLib headers.

Not attempted.

> But our root problem is our undisciplined use of #include.  Can we agree
> on a sane set of rules?  Here's my proposal:
>
> 1. Have a carefully curated header that's included everywhere first.  We
>    got that already thanks to Peter: osdep.h.
>
> 2. Headers should normally include everything they need beyond osdep.h.
>    If exceptions are needed for some reason, they must be documented in
>    the header.  If all that's needed from a header is typedefs, put
>    those into qemu/typedefs.h instead of including the header.
>
> 3. Cyclic inclusion is forbidden.
>
> Nice to have: "make check" checks 2. and 3.

See my "[RFC v4 0/7] Baby steps towards saner headers".

[...]

Headers that are prerequisites of more than 2000 .o:

   6312 config-host.h
   2827 include/block/aio.h
   4537 include/disas/dis-asm.h
   3634 include/exec/cpu-all.h
   5177 include/exec/cpu-common.h
   3634 include/exec/cpu-defs.h
   5401 include/exec/hwaddr.h
   5395 include/exec/memattrs.h
   5170 include/exec/memory.h
   3514 include/exec/memory_ldst.inc.h
   3514 include/exec/memory_ldst_cached.inc.h
   3514 include/exec/memory_ldst_phys.inc.h
   3517 include/exec/ramlist.h
   5699 include/fpu/softfloat-types.h
   6312 include/glib-compat.h
   5429 include/hw/hotplug.h
   2423 include/hw/hw.h
   5437 include/hw/irq.h
   5428 include/hw/qdev-core.h
   2520 include/hw/qdev-properties.h
   2297 include/hw/qdev.h
   2425 include/migration/qemu-file-types.h
   2564 include/migration/vmstate.h
   2502 include/qapi/error.h
   6063 include/qapi/util.h
   5939 include/qemu/atomic.h
   5494 include/qemu/bitmap.h
   5673 include/qemu/bitops.h
   5697 include/qemu/bswap.h
   6312 include/qemu/compiler.h
   3701 include/qemu/coroutine.h
   2838 include/qemu/event_notifier.h
   5683 include/qemu/host-utils.h
   4680 include/qemu/int128.h
   3701 include/qemu/lockable.h
   3187 include/qemu/log-for-trace.h
   2230 include/qemu/log.h
   2483 include/qemu/main-loop.h
   5568 include/qemu/module.h
   5137 include/qemu/notify.h
   6312 include/qemu/osdep.h
   5561 include/qemu/processor.h
   5560 include/qemu/qsp.h
   5863 include/qemu/queue.h
   5178 include/qemu/rcu.h
   5398 include/qemu/rcu_queue.h
   5178 include/qemu/sys_membarrier.h
   5560 include/qemu/thread-posix.h
   5560 include/qemu/thread.h
   4356 include/qemu/timer.h
   6312 include/qemu/typedefs.h
   2008 include/qemu/uuid.h
   4518 include/qom/cpu.h
   5537 include/qom/object.h
   6312 include/sysemu/os-posix.h
   2456 include/sysemu/reset.h
   2000 include/sysemu/sysemu.h
   6062 qapi/qapi-builtin-types.h
   2821 qapi/qapi-types-block-core.h
   2669 qapi/qapi-types-block.h
   3806 qapi/qapi-types-common.h
   2860 qapi/qapi-types-crypto.h
   2825 qapi/qapi-types-job.h
   3038 qapi/qapi-types-misc.h
   5249 qapi/qapi-types-run-state.h
   3148 qapi/qapi-types-sockets.h
   3634 tcg/i386/tcg-target.h
   3634 tcg/tcg-mo.h
   2369 trace/control-internal.h
   2369 trace/control.h
   2369 trace/event-internal.h
   2361 trace/ftrace.h
   5224 x86_64-softmmu/config-target.h

Bulky headers:

   size * count    size   count
       17744958   16461    1078 accel/tcg/tcg-runtime.h
       32519424   10304    6312 config-host.h
       21298950  525900      81 hw/display/trace.h
       25070220 1432584      35 hw/net/trace.h
       28700355  604218      95 hw/vfio/trace.h
       57913922   20486    2827 include/block/aio.h
       54939528   30744    1787 include/block/block.h
       15698796   53763     292 include/block/block_int.h
       12076546    6758    1787 include/block/dirty-bitmap.h
       90694630   19990    4537 include/disas/dis-asm.h
       22187844   68481     324 include/elf.h
       41198658   11337    3634 include/exec/cpu-all.h
       19118661    3693    5177 include/exec/cpu-common.h
       26052146    7169    3634 include/exec/cpu-defs.h
       10698472   11098     964 include/exec/cpu_ldst.h
       32181064   19144    1681 include/exec/exec-all.h
       14027000    2600    5395 include/exec/memattrs.h
      428019130   82789    5170 include/exec/memory.h
       12590662    3583    3514 include/exec/memory_ldst.inc.h
       13254808    3772    3514 include/exec/memory_ldst_cached.inc.h
       18258744    5196    3514 include/exec/memory_ldst_phys.inc.h
       37693186    6614    5699 include/fpu/softfloat-types.h
       40152640   41480     968 include/fpu/softfloat.h
       24408504    3867    6312 include/glib-compat.h
       16216423    2987    5429 include/hw/hotplug.h
       10178064    1872    5437 include/hw/irq.h
       27001315   27137     995 include/hw/pci/pci.h
       10509377   10541     997 include/hw/pci/pci_ids.h
       84058008   15486    5428 include/hw/qdev-core.h
       38099880   15119    2520 include/hw/qdev-properties.h
       10165600    4192    2425 include/migration/qemu-file-types.h
      140199520   54680    2564 include/migration/vmstate.h
       28062432   11216    2502 include/qapi/error.h
       14093499   21649     651 include/qapi/visitor.h
      122361217   20603    5939 include/qemu/atomic.h
       53242354    9691    5494 include/qemu/bitmap.h
       91409049   16113    5673 include/qemu/bitops.h
       82589409   14497    5697 include/qemu/bswap.h
       48009072    7606    6312 include/qemu/compiler.h
       32872282    8882    3701 include/qemu/coroutine.h
       20290224   11348    1788 include/qemu/hbitmap.h
       64189485   11295    5683 include/qemu/host-utils.h
       24059880    5141    4680 include/qemu/int128.h
       16473456    8474    1944 include/qemu/iov.h
       33191317   18553    1789 include/qemu/job.h
       11054887    2987    3701 include/qemu/lockable.h
       29294434   11798    2483 include/qemu/main-loop.h
       14081472    2529    5568 include/qemu/module.h
      120111048   19029    6312 include/qemu/osdep.h
       14389467    7383    1949 include/qemu/qht.h
      142840269   24363    5863 include/qemu/queue.h
       24735306    4777    5178 include/qemu/rcu.h
       68419650   12675    5398 include/qemu/rcu_queue.h
       59775560   10751    5560 include/qemu/thread.h
      122708520   28170    4356 include/qemu/timer.h
       26573520    4210    6312 include/qemu/typedefs.h
      166926546   36947    4518 include/qom/cpu.h
      336965209   60857    5537 include/qom/object.h
       55562856   55786     996 include/standard-headers/linux/pci_regs.h
       12834339   18361     699 include/sysemu/kvm.h
       19453584    3082    6312 include/sysemu/os-posix.h
       12720000    6360    2000 include/sysemu/sysemu.h
       16650270   26429     630 linux-user/qemu.h
       53241630   84110     633 linux-user/syscall_defs.h
       29144784 1214366      48 migration/trace.h
       16888732    5572    6062 qapi/qapi-builtin-types.h
      202683208  215544    2821 qapi/qapi-types-block-core.h
       11236490   12630    2669 qapi/qapi-types-block.h
       15791094   12447    3806 qapi/qapi-types-common.h
       21564400   22620    2860 qapi/qapi-types-crypto.h
       83229048   82188    3038 qapi/qapi-types-misc.h
       11551995   28665    1209 qapi/qapi-types-net.h
       25888068   14796    5249 qapi/qapi-types-run-state.h
       14465060   13785    3148 qapi/qapi-types-sockets.h
       18174224   62526     872 qapi/qapi-types-ui.h
       60562920  124872     485 target/arm/cpu.h
       18587552   63656     292 target/i386/cpu.h
       18575446   36139     514 target/mips/cpu.h
       44133336  109512     403 target/ppc/cpu.h
       28893934    7951    3634 tcg/i386/tcg-target.h
       24384936   51663     472 tcg/tcg-op.h
       18304270   11285    1622 tcg/tcg-opc.h
       79108184   48772    1622 tcg/tcg.h
       64600980   95352    1355 trace-dtrace-root.h
      233110135  344074    1355 trace-root.h
       22750450   33580    1355 trace-ust-root.h
       16220543    6847    2369 trace/control.h