From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Glen Choo <chooglen@google.com>
Subject: Re: [PATCH] CI: add SANITIZE=[address|undefined] jobs
Date: Fri, 29 Jul 2022 01:10:21 +0200 [thread overview]
Message-ID: <220729.8635ekkerp.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <YuL7EotrIpnOn5BT@coredump.intra.peff.net>
On Thu, Jul 28 2022, Jeff King wrote:
> On Thu, Jul 28, 2022 at 06:54:37PM +0200, Ævar Arnfjörð Bjarmason wrote:
> [...]
>> We also leave a lot of CI performance on the table by e.g. doing "chain
>> lint" in every single test run (except Windows), there *are* platform
>> edge-cases there like with SANITIZE=address, but I wonder if we should
>> just declare it good enough to do it in 1-2 jobs.
>
> I'd be fine with that, but I think chain lint isn't actually that
> expensive. The original in-shell bits are super cheap. The extra
> sed process is measurable, but I think I blunted the worst of it in the
> 2d86a96220 (t: avoid sed-based chain-linting in some expensive cases,
> 2021-05-13).
>
> Still, that patch should make it easy to time things just by setting
> GIT_TEST_CHAIN_LINT_HARDER=0 in various jobs. It does seem to buy ~100s
> of CPU time per test run on my Linux box. That's not a lot in the grand
> scheme, but perhaps adds up. And I could believe it's much worse on
> Windows. Maybe worth seeing how it performs in the actual CI
> environments.
>
>> Ditto TEST_NO_MALLOC_CHECK=1 & --no-bin-wrappers, but we can think about
>> all of those some other time....
>
> I'd be surprised if the malloc checking itself is all that expensive,
> though it does look like we call getconf and expr once per test there
> for setup. We could almost certainly hoist that out and call it once per
> script.
I posted some benchmarks for these a while ago:
https://lore.kernel.org/git/220405.86k0c3lt2l.gmgdl@evledraar.gmail.com/
YMMV, but these make a big difference for some tests, although less for
a whole run, as you point out disabling chain lint least impact.
For me a full test suite run is:
* TEST_NO_MALLOC_CHECK=1 --no-bin-wrappers --no-chain-lint:
real 1m55.600s
user 8m48.039s
sys 3m17.776s
* [no opts, TEST_NO_MALLOC_CHECK=0 & --bin-wrappers && --chain-lint are on]
real 2m18.126s
user 10m40.230s
sys 4m7.152s
So far from the >2x speedup you can get with some tests, but a big
difference nonetheless.
prev parent reply other threads:[~2022-07-28 23:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 11:09 [PATCH] CI: add SANITIZE=[address|undefined] jobs Ævar Arnfjörð Bjarmason
2022-07-26 13:30 ` Derrick Stolee
2022-07-26 13:33 ` Ævar Arnfjörð Bjarmason
2022-07-27 11:34 ` Derrick Stolee
2022-07-27 14:30 ` Junio C Hamano
2022-07-28 16:52 ` Ævar Arnfjörð Bjarmason
2022-07-28 17:41 ` Junio C Hamano
2022-07-27 19:18 ` Jeff King
2022-07-28 16:54 ` Ævar Arnfjörð Bjarmason
2022-07-28 21:09 ` Jeff King
2022-07-28 21:31 ` Jeff King
2022-07-28 23:10 ` Ævar Arnfjörð Bjarmason [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=220729.8635ekkerp.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.