From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH 1/5] t5309: run expected-to-fail `index-pack`s with `--threads=1`
Date: Wed, 10 Jan 2024 17:25:16 -0500 [thread overview]
Message-ID: <ZZ8ZTAj3E8eGJvDR@nand.local> (raw)
In-Reply-To: <xmqqfrz496ib.fsf@gitster.g>
On Wed, Jan 10, 2024 at 02:18:52PM -0800, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > But that requires us to tweak production code (albeit at a negligible
> > cost) in order to appease LSan in this narrow circumstance. Another
> > approach is to simply run these expected-to-fail `index-pack`
> > invocations with `--threads=1` so that we bypass the above issue
> > entirely.
>
> But of course, multi-threaded operation that production folks use
> will not be tested at all with the alternative.
Just the ones that we expect to fail *and* are in test scripts which are
marked as leak-free.
> > The downside of that approach is that the test doesn't match our
> > production code as well as it did before (where we might have run those
> > same `index-pack` invocations with >1 thread, depending on how many CPUs
> > the testing machine has). The risk there is that we might miss a
> > regression that would leave us in an inconsistent state. But that feels
> > rather unlikely in practice, and there are many other tests related to
> > `index-pack` in the suite.
>
> As long as "make sure we spawn all of them atmically" has negligible
> downside, I'd rather take that approach. Buying predictability with
> minimum cost is quite attractive.
Like I said earlier, I have no strong preference between either
approach. If you want to pick up Peff's patch instead of these five,
that is fine with me :-).
Thanks,
Taylor
next prev parent reply other threads:[~2024-01-10 22:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-05 8:50 [PATCH] index-pack: spawn threads atomically Jeff King
2024-01-05 16:33 ` Taylor Blau
2024-01-10 11:44 ` Jeff King
2024-01-10 17:34 ` Taylor Blau
2024-01-10 17:55 ` [PATCH 1/5] t5309: run expected-to-fail `index-pack`s with `--threads=1` Taylor Blau
2024-01-10 22:18 ` Junio C Hamano
2024-01-10 22:25 ` Taylor Blau [this message]
2024-01-10 17:55 ` [PATCH 2/5] t5302: " Taylor Blau
2024-01-10 17:55 ` [PATCH 3/5] t5308: " Taylor Blau
2024-01-10 17:55 ` [PATCH 4/5] t5313: " Taylor Blau
2024-01-10 17:55 ` [PATCH 5/5] t5325: " Taylor Blau
2024-01-11 6:53 ` [PATCH] index-pack: spawn threads atomically Jeff King
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=ZZ8ZTAj3E8eGJvDR@nand.local \
--to=me@ttaylorr.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).