From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Jeff King <peff@peff.net>, Eric Wong <e@80x24.org>,
Eric Sunshine via GitGitGadget <gitgitgadget@gmail.com>,
Git List <git@vger.kernel.org>, Elijah Newren <newren@gmail.com>,
Fabian Stelzer <fs@gigacodes.de>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 06/18] chainlint.pl: validate test scripts in parallel
Date: Tue, 22 Nov 2022 01:11:39 +0100 [thread overview]
Message-ID: <221122.86cz9fbyln.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <CAPig+cQEdidB4YHm9OiyOUe8mbTPBajjX5t-_6ZJVwRykXkqmg@mail.gmail.com>
On Mon, Nov 21 2022, Eric Sunshine wrote:
> On Mon, Nov 21, 2022 at 1:52 PM Jeff King <peff@peff.net> wrote:
>> On Mon, Nov 21, 2022 at 01:47:42PM -0500, Eric Sunshine wrote:
>> > I think Ævar's use-case for `make` parallelization was to speed up
>> > git-bisect runs. But thinking about it now, the likelihood of "lint"
>> > problems cropping up during a git-bisect run is effectively nil, in
>> > which case setting GIT_TEST_CHAIN_LINT=1 should be a perfectly
>> > appropriate way to take linting out of the equation when bisecting.
>>
>> Yes. It's also dumb to run a straight "make test" while bisecting in the
>> first place, because you are going to run a zillion tests that aren't
>> relevant to your bisection. Bisecting on "cd t && ./test-that-fails" is
>> faster, at which point you're only running the one lint process (and if
>> it really bothers you, you can disable chain lint as you suggest).
>
> I think I misspoke. Dredging up old memories, I think Ævar's use-case
> is that he now runs:
>
> git rebase -i --exec 'make test' ...
>
> in order to ensure that the entire test suite passes for _every_ patch
> in a series. (This is due to him having missed a runtime breakage by
> only running "make test" after the final patch in a series was
> applied, when the breakage was only temporary -- added by one patch,
> but resolved by some other later patch.)
>
> Even so, GIT_TEST_CHAIN_LINT=0 should be appropriate here too.
I'd like to make "make" fast in terms of avoiding its own overhead
before it gets to actual work mainly because of that use-case, but it
helps in general. E.g. if you switch branches we don't compile a file we
don't need to, we shouldn't re-run test checks we don't need either.
For t/ this is:
- Running chainlint.pl on the file, even if it didn't change
- Ditto check-non-portable-shell.pl
- Ditto "non-portable file name(s)" check
- Ditto "test -x" on all test files
I have a branch where these are all checked using dependencies instead,
e.g. we run a "test -x" on t0071-sort.sh and create a
".build/check-executable/t0071-sort.sh.ok" if that passed, we don't need
to shell out in the common case.
The results of that are, and this is a best case in picking one where
the test itself is cheap:
$ git hyperfine -L rev @{u},HEAD~,HEAD -s 'make CFLAGS=-O3' 'make test T=t0071-sort.sh' -w 1
Benchmark 1: make test T=t0071-sort.sh' in '@{u}
Time (mean ± σ): 1.168 s ± 0.074 s [User: 1.534 s, System: 0.082 s]
Range (min … max): 1.096 s … 1.316 s 10 runs
Benchmark 2: make test T=t0071-sort.sh' in 'HEAD~
Time (mean ± σ): 719.1 ms ± 46.1 ms [User: 910.6 ms, System: 79.7 ms]
Range (min … max): 682.0 ms … 828.2 ms 10 runs
Benchmark 3: make test T=t0071-sort.sh' in 'HEAD
Time (mean ± σ): 685.0 ms ± 34.2 ms [User: 645.0 ms, System: 56.8 ms]
Range (min … max): 657.6 ms … 773.6 ms 10 runs
Summary
'make test T=t0071-sort.sh' in 'HEAD' ran
1.05 ± 0.09 times faster than 'make test T=t0071-sort.sh' in 'HEAD~'
1.71 ± 0.14 times faster than 'make test T=t0071-sort.sh' in '@{u}'
The @{u} being "master", HEAD~ is "incremant without chainlint.pl", and
"HEAD" is where it's all incremental.
It's very WIP-quality, but I pushed the chainlint.pl part of it as a POC
just now, I did the others a while ago:
https://github.com/avar/git/tree/avar/t-Makefile-break-T-to-file-association
next prev parent reply other threads:[~2022-11-22 0:51 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-01 0:29 [PATCH 00/18] make test "linting" more comprehensive Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 01/18] t: add skeleton chainlint.pl Eric Sunshine via GitGitGadget
2022-09-01 12:27 ` Ævar Arnfjörð Bjarmason
2022-09-02 18:53 ` Eric Sunshine
2022-09-01 0:29 ` [PATCH 02/18] chainlint.pl: add POSIX shell lexical analyzer Eric Sunshine via GitGitGadget
2022-09-01 12:32 ` Ævar Arnfjörð Bjarmason
2022-09-03 6:00 ` Eric Sunshine
2022-09-01 0:29 ` [PATCH 03/18] chainlint.pl: add POSIX shell parser Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 04/18] chainlint.pl: add parser to validate tests Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 05/18] chainlint.pl: add parser to identify test definitions Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 06/18] chainlint.pl: validate test scripts in parallel Eric Sunshine via GitGitGadget
2022-09-01 12:36 ` Ævar Arnfjörð Bjarmason
2022-09-03 7:51 ` Eric Sunshine
2022-09-06 22:35 ` Eric Wong
2022-09-06 22:52 ` Eric Sunshine
2022-09-06 23:26 ` Jeff King
2022-11-21 4:02 ` Eric Sunshine
2022-11-21 13:28 ` Ævar Arnfjörð Bjarmason
2022-11-21 14:07 ` Eric Sunshine
2022-11-21 14:18 ` Ævar Arnfjörð Bjarmason
2022-11-21 14:48 ` Eric Sunshine
2022-11-21 18:04 ` Jeff King
2022-11-21 18:47 ` Eric Sunshine
2022-11-21 18:50 ` Eric Sunshine
2022-11-21 18:52 ` Jeff King
2022-11-21 19:00 ` Eric Sunshine
2022-11-21 19:28 ` Jeff King
2022-11-22 0:11 ` Ævar Arnfjörð Bjarmason [this message]
2022-09-01 0:29 ` [PATCH 07/18] chainlint.pl: don't require `return|exit|continue` to end with `&&` Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 08/18] t/Makefile: apply chainlint.pl to existing self-tests Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 09/18] chainlint.pl: don't require `&` background command to end with `&&` Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 10/18] chainlint.pl: don't flag broken &&-chain if `$?` handled explicitly Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 11/18] chainlint.pl: don't flag broken &&-chain if failure indicated explicitly Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 12/18] chainlint.pl: complain about loops lacking explicit failure handling Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 13/18] chainlint.pl: allow `|| echo` to signal failure upstream of a pipe Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 14/18] t/chainlint: add more chainlint.pl self-tests Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 15/18] test-lib: retire "lint harder" optimization hack Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 16/18] test-lib: replace chainlint.sed with chainlint.pl Eric Sunshine via GitGitGadget
2022-09-03 5:07 ` Elijah Newren
2022-09-03 5:24 ` Eric Sunshine
2022-09-01 0:29 ` [PATCH 17/18] t/Makefile: teach `make test` and `make prove` to run chainlint.pl Eric Sunshine via GitGitGadget
2022-09-01 0:29 ` [PATCH 18/18] t: retire unused chainlint.sed Eric Sunshine via GitGitGadget
2022-09-02 12:42 ` several messages Johannes Schindelin
2022-09-02 18:16 ` Eric Sunshine
2022-09-02 18:34 ` Jeff King
2022-09-02 18:44 ` Junio C Hamano
2022-09-11 5:28 ` [PATCH 00/18] make test "linting" more comprehensive Jeff King
2022-09-11 7:01 ` Eric Sunshine
2022-09-11 18:31 ` Jeff King
2022-09-12 23:17 ` Eric Sunshine
2022-09-13 0:04 ` 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=221122.86cz9fbyln.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=e@80x24.org \
--cc=fs@gigacodes.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).