Re: [PATCH 06/18] chainlint.pl: validate test scripts in parallel
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-11-22 00:51:40
On Mon, Nov 21 2022, Eric Sunshine wrote:
On Mon, Nov 21, 2022 at 1:52 PM Jeff King [off-list ref] wrote:quoted
On Mon, Nov 21, 2022 at 01:47:42PM -0500, Eric Sunshine wrote:quoted
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