Re: [PATCH v3 8/8] meson: precompile "git-compat-util.h"

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Apr 10, 2026 at 04:46:43PM +0100, Phillip Wood wrote:
> 
> 
> On 10/04/2026 16:36, SZEDER Gábor wrote:
> > On Thu, Mar 19, 2026 at 06:33:27AM +0100, Patrick Steinhardt wrote:
> > > Every compilation unit in Git is expected to include "git-compat-util.h"
> > > first, either directly or indirectly via "builtin.h". This header papers
> > > over differences between platforms so that we can expect the typical
> > > POSIX functions to exist. Furthermore, it provides functionality that we
> > > end up using everywhere.
> > > 
> > > This header is thus quite heavy as a consequence. Preprocessing it as a
> > > standalone unit via `clang -E git-compat-util.h` yields over 23,000
> > > lines of code overall. Naturally, it takes quite some time to compile
> > > all of this.
> > > 
> > > Luckily, this is exactly the kind of use case that precompiled headers
> > > aim to solve: instead of recompiling it every single time, we compile it
> > > once and then link the result into the executable. If include guards are
> > > set up properly it means that the file won't need to be reprocessed.
> > > 
> > > Set up such a precompiled header for "git-compat-util.h" and wire it up
> > > via Meson. This causes Meson to implicitly include the precompiled
> > > header in all compilation units. With GCC and Clang for example this is
> > > done via the "-include" statement [1].
> > > 
> > > This leads to a significant speedup when performing full builds:
> > > 
> > >    Benchmark 1: ninja (rev = HEAD~)
> > >    Time (mean ± σ):     14.467 s ±  0.126 s    [User: 248.133 s, System: 31.298 s]
> > >    Range (min … max):   14.195 s … 14.633 s    10 runs
> > > 
> > >    Benchmark 2: ninja (rev = HEAD)
> > >      Time (mean ± σ):     10.307 s ±  0.111 s    [User: 173.290 s, System: 23.998 s]
> > >      Range (min … max):   10.030 s … 10.433 s    10 runs
> > > 
> > >    Summary
> > >      ninja (rev = HEAD) ran
> > >        1.40 ± 0.02 times faster than ninja (rev = HEAD~)
> > > 
> > > [1]: https://gcc.gnu.org/onlinedocs/gcc/Precompiled-Headers.html
> > > 
> > > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > > ---
> > >   meson.build         | 2 ++
> > >   tools/precompiled.h | 1 +
> > >   2 files changed, 3 insertions(+)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index cd00be1c23..2002f4795e 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -1760,6 +1760,7 @@ libgit = declare_dependency(
> > >         c_args: libgit_c_args + [
> > >           '-DGIT_VERSION_H="' + version_def_h.full_path() + '"',
> > >         ],
> > > +      c_pch: 'tools/precompiled.h',
> > >         dependencies: libgit_dependencies,
> > >         include_directories: libgit_include_directories,
> > >       ),
> > 
> > Well, I don't do meson, but...
> > 
> > If I understand this right, this section here compiles all the source
> > files listed in "libgit_sources" using our new precompiled header.
> > But "libgit_sources" contains all source files under "reftable/",
> > which, with the sole exception of "reftable/system.c", don't include
> > "git-compat-util.h".
> > 
> > Now, building the reftable sources with "git-compat-util.h" included
> > through the precompiled header apparently didn't cause any compilation
> > errors...
> > But I think that's just accidental, and if a source file doesn't
> > include "git-compat-util.h", then it shouldn't be compiled with the
> > precompiled header.
> 
> FWIW I agree it would be better to keep the same includes when building with
> precompiled headers. There is some discussion about this starting at
> https://lore.kernel.org/git/2fe87868-dff5-4b3a-95e3-d4b6376b59ed@gmail.com/

I'm not sure that "oh look, it didn't break, so it must be good" is
the right approach...

And why was this not mentioned in the commit message?!  Sigh...