Re: [Qemu-devel] [PATCH v3 00/29] Tame a few "touch this, recompile the world" headers

[Philippe Mathieu-Daudé]

Hi Markus,

On 8/9/19 8:46 AM, Markus Armbruster wrote:
> We have quite a few "touch this, recompile the world" headers.  My
> "build everything" tree has some 6600 objects (not counting tests and
> objects that don't depend on qemu/osdep.h).  Touching any of 54
> headers triggers a recompile of more than half of them.
> 
> This series reduces them to 46.
> 
> Six of the 54 are included always by design, via qemu/osdep.h.  These
> are
> 
>     bld/config-host.h
>     include/glib-compat.h
>     include/qemu/compiler.h
>     include/qemu/osdep.h
>     include/qemu/typedefs.h
>     include/sysemu/os-posix.h
> 
> Additionally, osdep.h includes either include/exec/poison.h or
> bld/TARGET_DIR/config-target.h.
> 
> The seven headers this series improves to my satisfaction (for now)
> are
> 
>     bld/qapi/qapi-types-common.h
>     include/block/aio.h
>     include/hw/irq.h
>     include/qemu/event_notifier.h
>     include/qemu/main-loop.h
>     include/qemu/uuid.h
>     include/sysemu/sysemu.h
> 
> Of these, block/aio.h, qemu/main-loop.h and sysemu/sysemu.h are
> particular significant, as they in turn include numerous other
> headers.
> 
> The series makes real progress on a few more, but they're still bad:
> 
>     bld/qapi/qapi-types-run-state.h
>     include/qemu/timer.h
>     include/qom/cpu.h
>     include/disas/dis-asm.h
>     include/qemu/notify.h
>     include/qemu/atomic.h
> 
> Minor improvements:
> 
>     bld/qapi/qapi-builtin-types.h
>     bld/qapi/qapi-types-sockets.h
>     include/exec/cpu-common.h
>     include/exec/hwaddr.h
>     include/exec/memattrs.h
>     include/exec/memory.h
>     include/exec/memory_ldst.inc.h
>     include/exec/memory_ldst_cached.inc.h
>     include/exec/memory_ldst_phys.inc.h
>     include/exec/ramlist.h
>     include/fpu/softfloat-types.h
>     include/hw/hotplug.h
>     include/hw/qdev-core.h
>     include/qapi/util.h
>     include/qemu/bitmap.h
>     include/qemu/bitops.h
>     include/qemu/bswap.h
>     include/qemu/coroutine.h
>     include/qemu/host-utils.h
>     include/qemu/int128.h
>     include/qemu/lockable.h
>     include/qemu/module.h
>     include/qemu/processor.h
>     include/qemu/qsp.h
>     include/qemu/queue.h
>     include/qemu/rcu.h
>     include/qemu/rcu_queue.h
>     include/qemu/sys_membarrier.h
>     include/qemu/thread-posix.h
>     include/qemu/thread.h
>     include/qom/object.h
> 
> Untouched:
> 
>     include/exec/cpu-all.h
>     include/exec/cpu-defs.h
>     tcg/i386/tcg-target.h
>     tcg/tcg-mo.h
> 
> Further improvement is certainly possible.  exec/cpu-all.h,
> exec/cpu-defs.h, exec/memory.h, hw/qdev-core.h, qemu/coroutine.h,
> qemu/lockable.h, and qom/cpu.h each pull in more than ten other
> headers, which makes them particularly wortwhile targets.
> 
> Observed patterns of #include misuse:
> 
> * Copy pasta
> 
>   I found and deleted quite a few #include that were almost certainly
>   never needed.  The most likely explanation is lazy copying from a
>   "similar" file.  My deletions produced only minor improvements,
>   though.
> 
> * "Convenience" headers
> 
>   We sometimes have a header include a bunch of other headers the
>   header itself doesn't need, so the header's users don't have to.  An
>   extreme case is hw/hw.h: it pulls in more than 40 other headers,
>   then declares just hw_error().  Most of its users need only a
>   fraction of it.  PATCH 08-09,12-18 fix that, trading the very
>   occasional convenience of not having to type a few #include
>   directives for build speed.
> 
> * "Fat" headers
> 
>   Some headers provide many things to many customers.  Bad when the
>   customers generally need only parts.  Worse when such a "fat" header
>   pulls in loads more.  This series grapples with three instances:
>   qapi/qapi-types-common.h (PATCH 03), hw/boards.h, which pulls in
>   almost 70 headers (PATCH 19-23), and sysemu/sysemu.h, which pulls in
>   more than 20 (PATCH 23-28).
> 
> * Design erosion
> 
>   Off-the-cuff additions to headers can erode design.  For instance,
>   the generated trace.h were carefully designed for minimal
>   dependencies.  We let them balloon when we added the per-vCPU
>   tracing feature a few years later.  PATCH 07 grapples with that.

What can prevent us from these misuse patterns?

Will you redo this analysis/series after each releases?

How to automate misuse checks?

Can we report some metrics and warn if there a considerable discrepancy?