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?
