CI warnings test for 32 bit, and faster headerscheck

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: CI warnings test for 32 bit, and faster headerscheck
Date: 2024-10-03 05:56:24
Message-ID: CA+hUKGJjQyZUvcu6udk5OKz5rnaF4a_hm5nb_VtZHYMH+vsN0g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-ci-Use-ccache-for-headerscheck-cpluspluscheck.patch text/x-patch 3.1 KB
0002-ci-Add-a-warning-check-for-32-bit-builds.patch text/x-patch 1.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2024-10-03 06:22:24 Re: general purpose array_sort
Previous Message Shlok Kyal 2024-10-03 05:53:04 Re: long-standing data loss bug in initial sync of logical replication