mirror of
https://github.com/git/git
synced 2024-08-27 19:49:21 +00:00
![]() The preceding commits to make the "coccicheck" target incremental made it slower in some cases. As an optimization let's not have the many=many mapping of <*.cocci>=<*.[ch]>, but instead concat the <*.cocci> into an ALL.cocci, and then run one-to-many ALL.cocci=<*.[ch]>. A "make coccicheck" is now around 2x as fast as it was on "master", and around 1.5x as fast as the preceding change to make the run incremental: $ git hyperfine -L rev origin/master,HEAD~,HEAD -p 'make clean' 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' -r 3 Benchmark 1: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master Time (mean ± σ): 4.258 s ± 0.015 s [User: 27.432 s, System: 1.532 s] Range (min … max): 4.241 s … 4.268 s 3 runs Benchmark 2: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~ Time (mean ± σ): 5.365 s ± 0.079 s [User: 36.899 s, System: 1.810 s] Range (min … max): 5.281 s … 5.436 s 3 runs Benchmark 3: make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD Time (mean ± σ): 2.725 s ± 0.063 s [User: 14.796 s, System: 0.233 s] Range (min … max): 2.667 s … 2.792 s 3 runs Summary 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD' ran 1.56 ± 0.04 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'origin/master' 1.97 ± 0.05 times faster than 'make coccicheck SPATCH=spatch COCCI_SOURCES="$(echo $(ls o*.c builtin/h*.c))"' in 'HEAD~' This can be turned off with SPATCH_CONCAT_COCCI, but as the beneficiaries of "SPATCH_CONCAT_COCCI=" would mainly be those developing the *.cocci rules themselves, let's leave this optimization on by default. For more information see my "Optimizing *.cocci rules by concat'ing them" (<220901.8635dbjfko.gmgdl@evledraar.gmail.com>) on the cocci@inria.fr mailing list. This potentially changes the results of our *.cocci rules, but as noted in that discussion it should be safe for our use. We don't name rules, or if we do their names don't conflict across our *.cocci files. To the extent that we'd have any inter-dependencies between rules this doesn't make that worse, as we'd have them now if we ran "make coccicheck", applied the results, and would then have (due to hypothetical interdependencies) suggested changes on the subsequent "make coccicheck". Our "coccicheck-test" target makes use of the ALL.cocci when running tests, e.g. when testing unused.{c,out} we test it against ALL.cocci, not unused.cocci. We thus assert (to the extent that we have test coverage) that this concatenation doesn't change the expected results of running these rules. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> |
||
---|---|---|
.. | ||
tests | ||
.gitignore | ||
array.cocci | ||
commit.cocci | ||
equals-null.cocci | ||
flex_alloc.cocci | ||
free.cocci | ||
hashmap.cocci | ||
object_id.cocci | ||
preincr.cocci | ||
qsort.cocci | ||
README | ||
strbuf.cocci | ||
swap.cocci | ||
the_repository.pending.cocci | ||
unused.cocci | ||
xcalloc.cocci | ||
xopen.cocci | ||
xstrdup_or_null.cocci |
This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/) semantic patches that might be useful to developers. There are two types of semantic patches: * Using the semantic transformation to check for bad patterns in the code; The target 'make coccicheck' is designed to check for these patterns and it is expected that any resulting patch indicates a regression. The patches resulting from 'make coccicheck' are small and infrequent, so once they are found, they can be sent to the mailing list as per usual. Example for introducing new patterns: 67947c34ae (convert "hashcmp() != 0" to "!hasheq()", 2018-08-28) b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24) Example of fixes using this approach: 248f66ed8e (run-command: use strbuf_addstr() for adding a string to a strbuf, 2018-03-25) f919ffebed (Use MOVE_ARRAY, 2018-01-22) These types of semantic patches are usually part of testing, c.f. 0860a7641b (travis-ci: fail if Coccinelle static analysis found something to transform, 2018-07-23) * Using semantic transformations in large scale refactorings throughout the code base. When applying the semantic patch into a real patch, sending it to the mailing list in the usual way, such a patch would be expected to have a lot of textual and semantic conflicts as such large scale refactorings change function signatures that are used widely in the code base. A textual conflict would arise if surrounding code near any call of such function changes. A semantic conflict arises when other patch series in flight introduce calls to such functions. So to aid these large scale refactorings, semantic patches can be used. However we do not want to store them in the same place as the checks for bad patterns, as then automated builds would fail. That is why semantic patches 'contrib/coccinelle/*.pending.cocci' are ignored for checks, and can be applied using 'make coccicheck-pending'. This allows to expose plans of pending large scale refactorings without impacting the bad pattern checks. Git-specific tips & things to know about how we run "spatch": * The "make coccicheck" will piggy-back on "COMPUTE_HEADER_DEPENDENCIES". If you've built a given object file the "coccicheck" target will consider its depednency to decide if it needs to re-run on the corresponding source file. This means that a "make coccicheck" will re-compile object files before running. This might be unexpected, but speeds up the run in the common case, as e.g. a change to "column.h" won't require all coccinelle rules to be re-run against "grep.c" (or another file that happens not to use "column.h"). To disable this behavior use the "SPATCH_USE_O_DEPENDENCIES=NoThanks" flag. * To speed up our rules the "make coccicheck" target will by default concatenate all of the *.cocci files here into an "ALL.cocci", and apply it to each source file. This makes the run faster, as we don't need to run each rule against each source file. See the Makefile for further discussion, this behavior can be disabled with "SPATCH_CONCAT_COCCI=". But since they're concatenated any <id> in the <rulname> (e.g. "@ my_name", v.s. anonymous "@@") needs to be unique across all our *.cocci files. You should only need to name rules if other rules depend on them (currently only one rule is named).