Hi,
Today when "adder" choked on a compiler warning, I was annoyed that CI
knew about that[1] but didn't turn red because only the
CompilerWarnings task fails on warnings and it doesn't test 32 bit.
So here's a patch for that. Tom has already fixed that in master, but
my branch with this change triggered a failure[2] before the fix went
in.
Before adding another ~17s to every CI run (configure: ~9s, make: ~8s)
I figured I should optimise a nearby command that stands out as
wasting a huge amount of time, so that we come out ahead: headerscheck
and cpluspluscheck currently run in ~90s and ~45s respectively. If
you swizzle things around only slightly you can turn on ccache and get
them down to ~20s and ~05s, depending on ccache hit ratio.
The net result of both patches is that CompilerWarnings completes in 4
minutes[3] instead of a bit over 5, assuming ccache doesn't miss.
You could probably squeeze another few seconds out of headerscheck by
changing the loop to generate a script $name.sh for each header, where
each script constructs $name.c and invokes the compiler, and then use
something like find $tmp -name '*.sh' | xargs -P $(nproc) ... to
parallelise all the work, and finally collect all the results from
$tmp/*.output, but at a wild guess that could save only another ~5s or
so. Most of the available speedup comes from not compiling at all.
Hmm, given that configure uses more time than compiling (assuming 100%
ccache hits) and is woefully serial, I wonder what ingredients you'd
need to hash to have bulletproof cache invalidation for a persistent
configure cache, ie that survives between runs. Maybe something like
the output of "apt list --installed" (which lists installed Debian
packages and versions, so any library, tool etc change would
invalidate it, probably just when the CI images gets rebuilt
periodically) would be enough? Maybe we should change these over to
meson anyway, but then the same type of logic probably applies.
[1] https://cirrus-ci.com/task/5357841391812608?logs=build_32#L585
[2] https://cirrus-ci.com/task/5988615321288704
[3] https://cirrus-ci.com/task/5343255280222208
From ffdfd0fd0b0966b65e08e94e59f6d6e6cf685e30 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Thu, 3 Oct 2024 11:11:32 +1300
Subject: [PATCH 1/2] ci: Use ccache for headerscheck/cpluspluscheck.
Using stable pathnames for the generated files and turning on ccache
shaves about a minute and a half off typical runtime.
---
.cirrus.tasks.yml | 8 +++-----
src/tools/pginclude/headerscheck | 16 +++++++++++++---
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 90cb95c8681..1f67065816b 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -775,8 +775,6 @@ 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
# - XXX have to disable ICU to avoid errors:
# https://postgr.es/m/20220323002024.f2g6tivduzrktgfa%40alap3.anarazel.de
@@ -787,11 +785,11 @@ task:
${LINUX_CONFIGURE_FEATURES} \
--without-icu \
--quiet \
- CC="gcc" CXX"=g++" CLANG="clang-16"
+ CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang-16"
make -s -j${BUILD_JOBS} clean
- time make -s headerscheck EXTRAFLAGS='-fmax-errors=10'
+ time make -s headerscheck EXTRAFLAGS='-fmax-errors=10' TMPDIR="headerscheck_tmp"
headers_cpluspluscheck_script: |
- time make -s cpluspluscheck EXTRAFLAGS='-fmax-errors=10'
+ time make -s cpluspluscheck EXTRAFLAGS='-fmax-errors=10' TMPDIR="cpluspluscheck_tmp"
always:
upload_caches: ccache
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 3fc737d2cc1..b53a8648506 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -72,7 +72,14 @@ else
fi
# Create temp directory.
-tmp=`mktemp -d /tmp/$me.XXXXXX`
+if [ -z "$TMPDIR" ] ; then
+ tmp=`mktemp -d /tmp/$me.XXXXXX`
+else
+ # Use a directory name provided. It will be removed at end. A stable name
+ # is needed for ccache to be effective.
+ tmp="$TMPDIR"
+ mkdir -p "$tmp"
+fi
trap "ret=$?; rm -rf $tmp; exit $ret" 0 1 2 3 15
@@ -81,6 +88,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.
+ name=$(echo $f | tr "\/." "__")
+
# Ignore files that are unportable or intentionally not standalone.
# These files are platform-specific, and c.h will include the
@@ -227,7 +237,7 @@ do
esac
echo "#include \"$f\""
$cplusplus && echo '};'
- } >$tmp/test.$ext
+ } >$tmp/$name.$ext
# Some subdirectories need extra -I switches.
case "$f" in
@@ -249,7 +259,7 @@ 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/$name.$ext -o $tmp/$name.o
then
exit_status=1
fi
--
2.46.1
From ea67ed329ee2041a5817396dc9ea05c68882af65 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Thu, 3 Oct 2024 14:45:47 +1300
Subject: [PATCH 2/2] ci: Add a warning check for 32 bit builds.
We already compile and test 32 bit builds, but warnings don't turn
anything red unless something actually breaks at runtime. Add a new
command to the CompileWarnings task for that, so that such breakage is
less likely to make it to the build farm to be caught by "adder".
---
.cirrus.tasks.yml | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 1f67065816b..a9401977db6 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -234,6 +234,24 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
--with-uuid=ossp
--with-zstd
+LINUX_CONFIGURE_32_FEATURES: &LINUX_CONFIGURE_32_FEATURES >-
+ --with-gssapi
+ --with-icu
+ --with-ldap
+ --with-libxml
+ --with-libxslt
+ --with-llvm
+ --with-lz4
+ --with-pam
+ --with-perl
+ --with-python
+ --with-selinux
+ --with-ssl=openssl
+ --with-systemd
+ --with-tcl --with-tclconfig=/usr/lib/i386-linux-gnu/tcl8.6/
+ --with-uuid=e2fs
+ --with-zstd
+
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
-Duuid=e2fs
@@ -726,6 +744,21 @@ task:
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ # gcc, cassert on, 32 bit
+ always:
+ gcc_32_a_warning_script: |
+ time ./configure \
+ --cache gcc32.cache \
+ --enable-cassert \
+ ${LINUX_CONFIGURE_32_FEATURES} \
+ CC="ccache gcc -m32" \
+ CXX="ccache g++ -m32" \
+ CLANG="ccache clang-16 -m32" \
+ PERL="perl5.36-i386-linux-gnu" \
+ PKG_CONFIG_PATH="/usr/lib/i386-linux-gnu/pkgconfig"
+ make -s -j${BUILD_JOBS} clean
+ time make -s -j${BUILD_JOBS} world-bin
+
# clang, cassert off, dtrace off
always:
clang_warning_script: |
--
2.46.1
