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.
From b8454f9bb493220f011852fc2ac33e7c2cf02627 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Nov 2025 12:18:59 +0100
Subject: [PATCH v2 1/3] headerscheck ccache support
Currently, headerscheck and cpluspluscheck are very slow, and they
defeat use of ccache. This fixes that, and now they are much faster.
The problem was 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.
The solution is to create the test files in the build directory. For
example, for src/include/storage/ipc.h, we generate
tmp_headerscheck_c/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 subdirectory is used to keep the cleanup trap simple.)
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.
Co-authored-by: Thomas Munro <[email protected]>
Discussion:
https://www.postgresql.org/message-id/flat/b49e74d4-3cf9-4d1c-9dce-09f75e55d026%40eisentraut.org
---
.cirrus.tasks.yml | 5 ++---
src/tools/pginclude/headerscheck | 15 +++++++++++----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 2fe9671f3dc..038d043d00e 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -1005,16 +1005,15 @@ task:
###
# Verify headerscheck / cpluspluscheck succeed
#
- # - Don't use ccache, the files are uncacheable, polluting ccache's
- # cache
# - Use -fmax-errors, as particularly cpluspluscheck can be very verbose
###
always:
headers_headerscheck_script: |
time ./configure \
${LINUX_CONFIGURE_FEATURES} \
+ --cache gcc.cache \
--quiet \
- CC="gcc" CXX"=g++" CLANG="clang"
+ 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: |
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index bf4992e33ae..8caa6e4de12 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -73,8 +73,10 @@ else
COMPILER_FLAGS="$CPPFLAGS $CFLAGS $ICU_CFLAGS $LLVM_CPPFLAGS"
fi
-# Create temp directory.
-tmp=`mktemp -d /tmp/$me.XXXXXX`
+# Create temp directory. Help ccache by using a stable name. Include
+# extension to allow running C and C++ checks in parallel.
+tmp="tmp_${me}_${ext}"
+mkdir -p "$tmp"
trap "ret=$?; rm -rf $tmp; exit $ret" 0 1 2 3 15
@@ -83,6 +85,9 @@ exit_status=0
# Scan all of src/ and contrib/ for header files.
for f in `cd "$srcdir" && find src contrib -name '*.h' -print`
do
+ # Help ccache by using a stable name. Remove slashes and dots.
+ test_file_name=$(printf '%s' "$f" | tr '/.' '__')
+
# Ignore files that are unportable or intentionally not standalone.
# These files are platform-specific, and c.h will include the
@@ -232,7 +237,7 @@ do
esac
echo "#include \"$f\""
$cplusplus && echo '};'
- } >$tmp/test.$ext
+ } >$tmp/$test_file_name.$ext
# Some subdirectories need extra -I switches.
case "$f" in
@@ -254,10 +259,12 @@ 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 "$tmp/$test_file_name.$ext" -o
"$tmp/$test_file_name.o"
then
exit_status=1
fi
+
+ rm -f "$tmp/$test_file_name.$ext" "$tmp/$test_file_name.o"
done
exit $exit_status
--
2.52.0
From 0a580cb2e58dcc257978d5cc20528f2e4a315880 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Nov 2025 12:21:31 +0100
Subject: [PATCH v2 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'
always:
upload_caches: ccache
--
2.52.0
From 863b295a4be6304fe629082d8ae0a9ebf5c7d2eb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 28 Nov 2025 12:03:05 +0100
Subject: [PATCH v2 3/3] headerscheck: Parallelize by writing out a makefile
---
GNUmakefile.in | 4 ++--
src/tools/pginclude/headerscheck | 29 +++++++++++++++++++----------
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index cf6e759486e..eb36fd07f39 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -129,9 +129,9 @@ distcheck: dist
@echo "Distribution integrity checks out."
headerscheck: submake-generated-headers
- $(top_srcdir)/src/tools/pginclude/headerscheck $(top_srcdir)
$(abs_top_builddir)
+ +$(top_srcdir)/src/tools/pginclude/headerscheck $(top_srcdir)
$(abs_top_builddir)
cpluspluscheck: submake-generated-headers
- $(top_srcdir)/src/tools/pginclude/headerscheck --cplusplus
$(top_srcdir) $(abs_top_builddir)
+ +$(top_srcdir)/src/tools/pginclude/headerscheck --cplusplus
$(top_srcdir) $(abs_top_builddir)
.PHONY: dist distcheck docs install-docs world check-world install-world
installcheck-world headerscheck cpluspluscheck
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 8caa6e4de12..3d119c0b3c6 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -82,6 +82,17 @@ trap "ret=$?; rm -rf $tmp; exit $ret" 0 1 2 3 15
exit_status=0
+cat >"$tmp/Makefile" <<EOF
+all:
+
+EXTRAINCLUDES = $perl_includespec $python_includespec -I
$builddir/src/interfaces/ecpg/include -I $srcdir/src/interfaces/ecpg/include -I
$builddir/src/backend/parser/ -I $builddir/src/backend/utils/adt/
+
+%.o: %.$ext
+ $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 \$< -o \$@
+
+ALL_FILES =
+EOF
+
# Scan all of src/ and contrib/ for header files.
for f in `cd "$srcdir" && find src contrib -name '*.h' -print`
do
@@ -255,16 +266,14 @@ do
EXTRAINCLUDES="" ;;
esac
- # Run the test.
- 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_file_name.$ext" -o
"$tmp/$test_file_name.o"
- then
- exit_status=1
- fi
-
- rm -f "$tmp/$test_file_name.$ext" "$tmp/$test_file_name.o"
+ printf 'ALL_FILES += %s\n' "$test_file_name.$ext" >> "$tmp/Makefile"
done
+cat >>"$tmp/Makefile" <<EOF
+
+all: \$(ALL_FILES:%.$ext=%.o)
+EOF
+
+make -C "$tmp" -k -s all
+
exit $exit_status
--
2.52.0
