On Thu, Oct 20, 2022 at 10:59:28PM -0700, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > You can make it a bit faster by running both at once as
> > SANITIZE=address,undefined.
>
> With a single combined sanitier job, we saw failures at CI of p4
> tests, where the server "goes away" in the middle of tests, for
> apparently no reason, and I just speculated perhaps the tests taking
> too long may be causing an impatient server side to go away before
> the client side finishes its work or something. After splitting the
> ASan and UBSan jobs into two and ejecting a topic out of 'seen', CI
> started passing for the first time in several days.
I am not too surprised to hear that. We flushed out many test races in
the past from using valgrind and SANITIZE. But I don't run the p4 tests
locally, so this is likely some of their first exposure to slower runs.
But if that is the case, then you are probably just papering over
failures here which will likely come back, at least racily.
I admit I was surprised how much slower the combined one is. Here are a
few timings on my laptop:
$ for i in '' address undefined address,undefined; do
echo -n "SANITIZE=$i"
make SANITIZE=$i >/dev/null 2>&1 &&
(cd t && time ./t3700-add.sh >/dev/null 2>&1)
echo
done
SANITIZE=
real 0m1.109s
user 0m0.533s
sys 0m0.589s
SANITIZE=address
real 0m1.816s
user 0m0.999s
sys 0m0.806s
SANITIZE=undefined
real 0m1.336s
user 0m0.618s
sys 0m0.710s
SANITIZE=address,undefined
real 0m2.928s
user 0m1.304s
sys 0m1.635s
Curiously, with CC=clang the results are closer to what I'd expect:
SANITIZE=
real 0m1.186s
user 0m0.608s
sys 0m0.579s
SANITIZE=address
real 0m1.910s
user 0m1.014s
sys 0m0.890s
SANITIZE=undefined
real 0m1.477s
user 0m0.611s
sys 0m0.865s
SANITIZE=address,undefined
real 0m2.309s
user 0m1.126s
sys 0m1.191s
I'm not sure what the takeaway is. If using two jobs produces
appreciably fewer races in practice, it may be worth doing. But I wonder
if just using clang would work better.
-Peff
