headerscheck ccache support

On 2025-Nov-21, Peter Eisentraut wrote:

Currently, headerscheck and cpluspluscheck are very slow, and they defeat
use of ccache. I have fixed that, and now they are much faster. :-)

Yeah, I had noticed this too. Thanks for fixing it.

My solution is to create the test files in the build directory. For
example, for src/include/storage/ipc.h I generate

headerscheck_src_include_storage_ipc_h.c (or .cpp)

Now ccache works.

Sounds reasonable. I notice that you're cleaning this file in a `rm`
line in the loop,

@@ -253,10 +249,11 @@ do
if ! $COMPILER $COMPILER_FLAGS -I $builddir -I $srcdir \
-I $builddir/src/include -I $srcdir/src/include \
-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
-		$EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.$ext -o $tmp/test.o
+		$EXTRAINCLUDES $EXTRAFLAGS -c $test_file_name.$ext -o $test_file_name.o
then
exit_status=1
fi
+	rm -f "$test_file_name.$ext" "$test_file_name.o"
done

but this means that if the script is interrupted halfway through, one
file or two files might remain in place. Would it be possible to have
the current file name in a variable, so that the `trap` line can delete
them?

I've been also wondering about testing whether `parallel` is installed,
and use that if so.

# Verify headerscheck / cpluspluscheck succeed
#
- # - Don't use ccache, the files are uncacheable, polluting ccache's
- # cache

So how bad is the effect of the cache pollution that's now going to
occur? I imagine we don't really care. (I have no idea how to measure
this, and probably it's a waste of time just to try.)

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"E pur si muove" (Galileo Galilei)

Hi,

On 2025-11-21 11:48:10 +0100, Peter Eisentraut wrote:

Currently, headerscheck and cpluspluscheck are very slow, and they defeat
use of ccache. I have fixed that, and now they are much faster. :-)

The problem was (I think) that the test files are created in a
randomly-named directory (`mktemp -d /tmp/$me.XXXXXX`), and this directory
is named on the compiler command line, which is part of the cache key.

My solution is to create the test files in the build directory. For
example, for src/include/storage/ipc.h I generate

headerscheck_src_include_storage_ipc_h.c (or .cpp)

Now ccache works. (And it's also a bit easier to debug everything with this
naming.)

I applaud this effort. I think long-term I want this stuff properly
integrated into the build system, but this is a good step on its own.

The observed speedup on Cirrus CI for headerscheck plus cpluspluscheck is
from about 1min 20s to only 20s. In local use, the speedups are similar.

(I noticed that on Cirrus CI, the first uncached run with this patch was
quite a bit slower. I don't know if this was because of the additional
cache population that was happening, or if it was a fluke. Maybe others can
try it in their environments. Maybe it's not a problem in the long run.)

That doesn't surprise me - one of the major throttling factors on CI is the
number of iops. Filling an empty cache drastically increases iops, due to
having to create all the cache files (there are multiple for each cache
entry).

Hm. I wonder whether we would gain performance if we changed the linux CI
image to use data=writeback instead of the default data=ordered, that can
really help with metadata heavy workloads.

Bilal, any chance you would try modifying the CI image generation to change
the default option for the root filesystem using tune2fs -o
journal_data_writeback (that's easier IME than changing the mount option, as
the root filesystem is mounted very early, in the initramfs, before fstab is
known). Or perhaps just do it interactively in a VM (needs a reboot though)?

Greetings,

Andres Freund

Hi,

On 2025-11-21 13:14:18 +0100, �lvaro Herrera wrote:

On 2025-Nov-21, Peter Eisentraut wrote:

# Verify headerscheck / cpluspluscheck succeed
#
- # - Don't use ccache, the files are uncacheable, polluting ccache's
- # cache

So how bad is the effect of the cache pollution that's now going to
occur? I imagine we don't really care. (I have no idea how to measure
this, and probably it's a waste of time just to try.)

I don't think there's any cache pollution after this change - the pollution
the comment was referencing was that ccache's cache would be filled with
entries for files that would *never* be used and would therefore be
useless. With this change the added use of ccache will just accelerate the CI
runs, which I wouldn't consider polluting...

Greetings,

Andres Freund

On 2025-Nov-21, Andres Freund wrote:

On 2025-11-21 13:14:18 +0100, Álvaro Herrera wrote:

So how bad is the effect of the cache pollution that's now going to
occur?

I don't think there's any cache pollution after this change - the
pollution the comment was referencing was that ccache's cache would be
filled with entries for files that would *never* be used and would
therefore be useless.

Ah, that makes sense.

With this change the added use of ccache will just accelerate the CI
runs, which I wouldn't consider polluting...

Sure.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"If you have nothing to say, maybe you need just the right tool to help you
not say it." (New York Times, about Microsoft PowerPoint)

On Fri, Nov 21, 2025 at 11:48 PM Peter Eisentraut <peter@eisentraut.org> wrote:

Currently, headerscheck and cpluspluscheck are very slow, and they
defeat use of ccache. I have fixed that, and now they are much faster. :-)

The problem was (I think) that the test files are created in a
randomly-named directory (`mktemp -d /tmp/$me.XXXXXX`), and this
directory is named on the compiler command line, which is part of the
cache key.

My solution is to create the test files in the build directory. For
example, for src/include/storage/ipc.h I generate

headerscheck_src_include_storage_ipc_h.c (or .cpp)

Now ccache works. (And it's also a bit easier to debug everything with
this naming.)

The observed speedup on Cirrus CI for headerscheck plus cpluspluscheck
is from about 1min 20s to only 20s. In local use, the speedups are similar.

+1

I wrote an almost identical patch[1]/messages/by-id/CA+hUKGJjQyZUvcu6udk5OKz5rnaF4a_hm5nb_VtZHYMH+vsN0g@mail.gmail.com and then lost it down the back of
the sofa. I was wondering about parallelising it next...

[1]: /messages/by-id/CA+hUKGJjQyZUvcu6udk5OKz5rnaF4a_hm5nb_VtZHYMH+vsN0g@mail.gmail.com

On 22.11.25 09:54, Thomas Munro wrote:

On Fri, Nov 21, 2025 at 11:48 PM Peter Eisentraut <peter@eisentraut.org> wrote:

Currently, headerscheck and cpluspluscheck are very slow, and they
defeat use of ccache. I have fixed that, and now they are much faster. :-)

The problem was (I think) that the test files are created in a
randomly-named directory (`mktemp -d /tmp/$me.XXXXXX`), and this
directory is named on the compiler command line, which is part of the
cache key.

My solution is to create the test files in the build directory. For
example, for src/include/storage/ipc.h I generate

headerscheck_src_include_storage_ipc_h.c (or .cpp)

Now ccache works. (And it's also a bit easier to debug everything with
this naming.)

The observed speedup on Cirrus CI for headerscheck plus cpluspluscheck
is from about 1min 20s to only 20s. In local use, the speedups are similar.

+1

I wrote an almost identical patch[1] and then lost it down the back of
the sofa. I was wondering about parallelising it next...

[1] /messages/by-id/CA+hUKGJjQyZUvcu6udk5OKz5rnaF4a_hm5nb_VtZHYMH+vsN0g@mail.gmail.com

Ah yes, that's about the same idea. The difference is that yours
requires specifying TMPDIR on the make invocation. So it wouldn't
happen by default for local (non-CI) use. I think I would like an
implementation that also worked out of the box locally.

On 21.11.25 13:14, Álvaro Herrera wrote:

Now ccache works.

Sounds reasonable. I notice that you're cleaning this file in a `rm`
line in the loop,

@@ -253,10 +249,11 @@ do
if ! $COMPILER $COMPILER_FLAGS -I $builddir -I $srcdir \
-I $builddir/src/include -I $srcdir/src/include \
-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
-		$EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.$ext -o $tmp/test.o
+		$EXTRAINCLUDES $EXTRAFLAGS -c $test_file_name.$ext -o $test_file_name.o
then
exit_status=1
fi
+	rm -f "$test_file_name.$ext" "$test_file_name.o"
done

but this means that if the script is interrupted halfway through, one
file or two files might remain in place. Would it be possible to have
the current file name in a variable, so that the `trap` line can delete
them?

Here is another patch set. I have made some tweaks to address the issue
you raise, and I took some code and inspiration from Thomas Munro's
patch. The solution I chose is to create a temporary subdirectory in
the build directory, and create the test files in there. That way the
trap can just blow away the directory, as before.

I've been also wondering about testing whether `parallel` is installed,
and use that if so.

Another approach I had in mind for some time is to just write out a
makefile with the test compile commands, and run that with make -j.
Demo patch attached. (I'm not seriously proposing this. For one thing,
we probably wouldn't want to introduce a dependency on make. But you
could probably write an equivalent ninja.build file.)

But this doesn't seem to buy very much. The overhead of the shell
script to write out the test files appears to become significant
compared the the actual compile commands.

Another simple idea is to run headerscheck and cpluspluscheck in
parallel. You can already do that manually, and we could do that on CI
to save about 50% wall-clock time. Patch attached.

Hi

On Fri, 28 Nov 2025 at 14:39, Peter Eisentraut <peter@eisentraut.org> wrote:

On 21.11.25 13:14, Álvaro Herrera wrote:

Now ccache works.

Sounds reasonable. I notice that you're cleaning this file in a `rm`
line in the loop,

@@ -253,10 +249,11 @@ do
if ! $COMPILER $COMPILER_FLAGS -I $builddir -I $srcdir \
-I $builddir/src/include -I $srcdir/src/include \
-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
-            $EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.$ext -o $tmp/test.o
+            $EXTRAINCLUDES $EXTRAFLAGS -c $test_file_name.$ext -o $test_file_name.o
then
exit_status=1
fi
+    rm -f "$test_file_name.$ext" "$test_file_name.o"
done

but this means that if the script is interrupted halfway through, one
file or two files might remain in place. Would it be possible to have
the current file name in a variable, so that the `trap` line can delete
them?

Here is another patch set.

I could not apply patches cleanly. Am I missing something?

$ git am ~/Downloads/v2-0001-headerscheck-ccache-support.patch
Applying: headerscheck ccache support
error: patch failed: src/tools/pginclude/headerscheck:73
error: src/tools/pginclude/headerscheck: patch does not apply
Patch failed at 0001 headerscheck ccache support

--
Regards,
Nazir Bilal Yavuz
Microsoft

On 2025-Nov-28, Nazir Bilal Yavuz wrote:

I could not apply patches cleanly. Am I missing something?

Yeah, I couldn't get `git am` or `git apply` to accept the patches
either, not even with -3. However, `patch -p1` does accept it. Weird.

I have git 2.47.3 and the patch says 2.52.0. Maybe somebody submitted
vibe-coded features into recent git, causing it to generate subtly
broken patch files?

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Para tener más hay que desear menos"

On 28.11.25 13:59, Álvaro Herrera wrote:

On 2025-Nov-28, Nazir Bilal Yavuz wrote:

I could not apply patches cleanly. Am I missing something?

Yeah, I couldn't get `git am` or `git apply` to accept the patches
either, not even with -3. However, `patch -p1` does accept it. Weird.

I had another commit in my local branch that I needed to get the actual
compilations working, but which was unrelated to this discussion.
Apparently, this created enough fuzz to break the subsequent patches.
Here is the patch series again completely, but ignore the 0000 patch for
now.

On 2025-Nov-28, Peter Eisentraut wrote:

Here is another patch set. I have made some tweaks to address the issue you
raise, and I took some code and inspiration from Thomas Munro's patch. The
solution I chose is to create a temporary subdirectory in the build
directory, and create the test files in there. That way the trap can just
blow away the directory, as before.

I tried with all patches applied, and it seems to work okay -- the
header compiles are all cached after the first pass, according to
ccache --show-stats.

Another approach I had in mind for some time is to just write out a makefile
with the test compile commands, and run that with make -j. Demo patch
attached. (I'm not seriously proposing this. For one thing, we probably
wouldn't want to introduce a dependency on make. But you could probably
write an equivalent ninja.build file.)

But this doesn't seem to buy very much. The overhead of the shell script to
write out the test files appears to become significant compared the the
actual compile commands.

Really? I tried editing the make line to have -j8 (your patch doesn't
have a -j switch at all there) and it runs in 4s instead of 12s on my
laptop, which I would call a significant win.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
#error "Operator lives in the wrong universe"
("Use of cookies in real-time system development", M. Gleixner, M. Mc Guire)

On 2025-Nov-28, Peter Eisentraut wrote:

But this doesn't seem to buy very much. The overhead of the shell script to
write out the test files appears to become significant compared the the
actual compile commands.

If you wanted to save some shell execution time, you could move the `tr`
calls to the bottom of the loop to avoid doing it for files that the
`if` block is going to discard. But is that significant? I doubt it.

(I didn't quite understand why you use printf instead of echo, given
that both are shell builtins in any case.)

I think parallelism is also going to win in the case where the compiles
are not cached.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/

On 28.11.25 14:10, Peter Eisentraut wrote:

On 28.11.25 13:59, Álvaro Herrera wrote:

On 2025-Nov-28, Nazir Bilal Yavuz wrote:

I could not apply patches cleanly. Am I missing something?

Yeah, I couldn't get `git am` or `git apply` to accept the patches
either, not even with -3.  However, `patch -p1` does accept it.  Weird.

I had another commit in my local branch that I needed to get the actual
compilations working, but which was unrelated to this discussion.
Apparently, this created enough fuzz to break the subsequent patches.
Here is the patch series again completely, but ignore the 0000 patch for
now.

I have committed patches 0000 and 0001, which was the main subject of
this thread.

What do people think about patch 0002, which runs headerscheck and
cpluspluscheck in parallel on ci? It should save several seconds of
wall-clock time for that task, and I don't see any drawbacks, unless you
want to retain the specific previous output format for some reason.

I don't plan to pursue the other parallelization work (patch 0003) at
this time.

On 28.11.25 14:29, Álvaro Herrera wrote:

Another approach I had in mind for some time is to just write out a makefile
with the test compile commands, and run that with make -j. Demo patch
attached. (I'm not seriously proposing this. For one thing, we probably
wouldn't want to introduce a dependency on make. But you could probably
write an equivalent ninja.build file.)

But this doesn't seem to buy very much. The overhead of the shell script to
write out the test files appears to become significant compared the the
actual compile commands.

Really? I tried editing the make line to have -j8 (your patch doesn't
have a -j switch at all there)

Note that the "+" I added to the targets in the top-level GNUmakefile.in
causes the make flags to passed down, so you can run make -jN
headerscheck etc. without having to edit or hardcode the make command
inside the script.

On 28.11.25 15:16, Álvaro Herrera wrote:

On 2025-Nov-28, Peter Eisentraut wrote:

But this doesn't seem to buy very much. The overhead of the shell script to
write out the test files appears to become significant compared the the
actual compile commands.

If you wanted to save some shell execution time, you could move the `tr`
calls to the bottom of the loop to avoid doing it for files that the
`if` block is going to discard. But is that significant? I doubt it.

This actually made a measurable difference, so I included that change in
the committed patch.

(I didn't quite understand why you use printf instead of echo, given
that both are shell builtins in any case.)

printf is nowadays preferable over echo in portable shell scripts. See
for example

https://cgit.git.savannah.gnu.org/cgit/autoconf.git/tree/NEWS#n435

and

https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/html_node/Limitations-of-Builtins.html

Hi,

On 2025-12-04 11:52:07 +0100, Peter Eisentraut wrote:

What do people think about patch 0002, which runs headerscheck and
cpluspluscheck in parallel on ci? It should save several seconds of
wall-clock time for that task, and I don't see any drawbacks, unless you
want to retain the specific previous output format for some reason.

I think the output today is easier to parse, it's more obvious whether the
error is from a cpluspluscheck violation or a headercheck violation. However,
the runtime win seems to more than outweigh that.

From 0a580cb2e58dcc257978d5cc20528f2e4a315880 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 28 Nov 2025 12:21:31 +0100
Subject: [PATCH v2.1 2/3] ci: Run headerscheck and cplusplucheck in parallel

---
.cirrus.tasks.yml | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 038d043d00e..69224fcfec7 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -1015,9 +1015,7 @@ task:
--quiet \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
-      time make -s headerscheck EXTRAFLAGS='-fmax-errors=10'
-    headers_cpluspluscheck_script: |
-      time make -s cpluspluscheck EXTRAFLAGS='-fmax-errors=10'
+      time make -s -j${BUILD_JOBS} -k -Otarget headerscheck cpluspluscheck EXTRAFLAGS='-fmax-errors=10'

Doesn't really matter, but I'd probably use ${CHECKFLAGS} instead of -Otarget
directly.

I'd add a comment saying that we run both in the same script to increase
parallelism and that we use -k to get the result of both. But again, this is
just a very minor nitpick, and if you prefer not to, I'm fine.

Greetings,

Andres Freund