[PATCH v4 00/12] cocci: make "incremental" possible + a ccache-like tool
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Oct 26, 2022, 14:20 UTC
- Message-ID
- <cover-v4-00.12-00000000000-20221026T141005Z-avarab@gmail.com>
- In-Reply-To
- <cover-v3-00.11-00000000000-20221014T152552Z-avarab@gmail.com>
A re-roll of the series to have "make coccicheck" run incrementally. For the v1 and general overview see: https://lore.kernel.org/git/cover-0.5-00000000000-20220825T141212Z-avarab@gmail.com/
For me a "make coccicheck" on master takes ~7m, with "SPATCH_BATCH_SIZE=0" (the usual way to speed it up on a beefy box) it's ~6m.
With this topic it takes 2m30s with no caching, and using the "spatchcache" tool in 12/12 10s. The ~6-7m to ~2m30s speed-up is mostly 11/12, i.e. we now concatenate the rules.
But being able to run this in ~10s once you set up "spatchcache" means that incremental runs become viable (e.g. sanity checks when merging topics in a loop).
These numbers are on a 8 core colo'd box, so YMMV. For GitHub CI the "static-analysis" (which has more than just "make coccicheck") now takes ~8m instead of ~15m.
Changes since v4:
* update contrib/coccinelle/README as we go, see the range-diff below.
* Added a 10/12 to remove named rules that didn't need their names, as we make an ALL.cocci these are more likely to collide.
* I made the spatchcache tool faster, it spend ~40ms per run on a cache hit, mostly on running "git config", now it can be told via environment variables to use its default, and not ask the config, which makes it take ~10ms on a cache hit.
* We now explain in 11/12's README addition that rule names are now global, as we concatenate all the *.cocci files.
Ævar Arnfjörð Bjarmason (12): Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T) cocci rules: remove unused "F" metavariable from pending rule Makefile: add ability to TAB-complete cocci *.patch rules Makefile: have "coccicheck" re-run if flags change Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading cocci: split off include-less "tests" from SPATCH_FLAGS cocci: split off "--all-includes" from SPATCH_FLAGS cocci: make "coccicheck" rule incremental cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES cocci rules: remove <id>'s from rules that don't need them cocci: run against a generated ALL.cocci spatchcache: add a ccache-alike for "spatch"
.gitignore | 1 + Makefile | 159 +++++++-- contrib/coccinelle/.gitignore | 1 + contrib/coccinelle/README | 49 +++ contrib/coccinelle/hashmap.cocci | 2 +- contrib/coccinelle/preincr.cocci | 2 +- contrib/coccinelle/spatchcache | 304 ++++++++++++++++++ contrib/coccinelle/strbuf.cocci | 2 +- contrib/coccinelle/swap.cocci | 2 +- .../coccinelle/the_repository.pending.cocci | 1 - shared.mak | 10 +- 11 files changed, 501 insertions(+), 32 deletions(-) create mode 100755 contrib/coccinelle/spatchcache
Range-diff against v3:
1: 4494c91df9a = 1: c0306e3c397 Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T)
2: 8219b1b12f2 = 2: 14e757cc020 cocci rules: remove unused "F" metavariable from pending rule
3: 6dbfafa08fd = 3: 210dbc6b101 Makefile: add ability to TAB-complete cocci *.patch rules
4: f779a2d22aa = 4: b859dae7509 Makefile: have "coccicheck" re-run if flags change
5: ab25b586f38 ! 5: 11b9227722b Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading
@@ Commit message
preceded it. As subsequent commits will add such comments we need to
split the existing comment up.
+ The wrapping for the "SPATCH_BATCH_SIZE" is now a bit odd, but
+ minimizes the diff size. As a subsequent commit will remove that
+ feature altogether this is worth it.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Makefile ##
6: 691be73b6fb = 6: 1c30f4b543b cocci: split off include-less "tests" from SPATCH_FLAGS
7: 2ca5ea5beca = 7: 86827388627 cocci: split off "--all-includes" from SPATCH_FLAGS
8: 2072a508064 = 8: 340ce523436 cocci: make "coccicheck" rule incremental
9: 739652eada9 ! 9: 134b3743a60 cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES
@@ Makefile: endef
endef
ifdef COCCI_GOALS
+
+ ## contrib/coccinelle/README ##
+@@ contrib/coccinelle/README: There are two types of semantic patches:
+
+ 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.
-: ----------- > 10: 89f778ac055 cocci rules: remove <id>'s from rules that don't need them
10: 52177ea2a68 ! 11: a848d09527f cocci: run against a generated ALL.cocci
@@ contrib/coccinelle/.gitignore
+/ALL.cocci
*.patch*
+ ## contrib/coccinelle/README ##
+@@ contrib/coccinelle/README: Git-specific tips & things to know about how we run "spatch":
+
+ 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).
+
## shared.mak ##
@@ shared.mak: ifndef V
## Used in "Makefile": SPATCH
11: f7ca3f9c9af ! 12: a728aa301a6 spatchcache: add a ccache-alike for "spatch"
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## contrib/coccinelle/README ##
+@@ contrib/coccinelle/README: Git-specific tips & things to know about how we run "spatch":
+ 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).
++
++ * To speed up incremental runs even more use the "spatchcache" tool
++ in this directory as your "SPATCH". It aimns to be a "ccache" for
++ coccinelle, and piggy-backs on "COMPUTE_HEADER_DEPENDENCIES".
++
++ It caches in Redis by default, see it source for a how-to.
++
++ In one setup with a primed cache "make coccicheck" followed by a
++ "make clean && make" takes around 10s to run, but 2m30s with the
++ default of "SPATCH_CONCAT_COCCI=Y".
++
++ With "SPATCH_CONCAT_COCCI=" the total runtime is around ~6m, sped
++ up to ~1m with "spatchcache".
++
++ Most of the 10s (or ~1m) being spent on re-running "spatch" on
++ files we couldn't cache, as we didn't compile them (in contrib/*
++ and compat/* mostly).
++
++ The absolute times will differ for you, but the relative speedup
++ from caching should be on that order.
+
## contrib/coccinelle/spatchcache (new) ##
@@
+#!/bin/sh
@@ contrib/coccinelle/spatchcache (new)
+# driving "spatch" in git.git, in particular if we invoke:
+#
+# make
++# # See "spatchCache.cacheWhenStderr" for why "--very-quiet" is
++# # used
+# make coccicheck SPATCH_FLAGS=--very-quiet
+#
+# We can with COMPUTE_HEADER_DEPENDENCIES (auto-detected as true with
@@ contrib/coccinelle/spatchcache (new)
+# spatchCache.{trace,debug} to debug whether something is set in the
+# cache, as we'll write to the spatch logs in .build/* we'd otherwise
+# always emit a NOCACHE.
++#
++# Reading the config can make the command much slower, to work around
++# this the config can be set in the environment, with environment
++# variable name corresponding to the config key. "default" can be used
++# to use whatever's the script default, e.g. setting
++# spatchCache.cacheWhenStderr=true and deferring to the defaults for
++# the rest is:
++#
++# export GIT_CONTRIB_SPATCHCACHE_DEBUG=default
++# export GIT_CONTRIB_SPATCHCACHE_TRACE=default
++# export GIT_CONTRIB_SPATCHCACHE_CACHEWHENSTDERR=true
++# export GIT_CONTRIB_SPATCHCACHE_SPATCH=default
++# export GIT_CONTRIB_SPATCHCACHE_DEPENDFORMAT=default
++# export GIT_CONTRIB_SPATCHCACHE_SETCMD=default
++# export GIT_CONTRIB_SPATCHCACHE_GETCMD=default
+
+set -e
+
++env_or_config () {
++ env="$1"
++ shift
++ if test "$env" = "default"
++ then
++ # Avoid expensive "git config" invocation
++ return
++ elif test -n "$env"
++ then
++ echo "$env"
++ else
++ git config $@ || :
++ fi
++}
++
+## Our own configuration & options
-+debug=$(git config --bool "spatchCache.debug")
++debug=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_DEBUG" --bool "spatchCache.debug")
+if test "$debug" != "true"
+then
+ debug=
@@ contrib/coccinelle/spatchcache (new)
+ set -x
+fi
+
-+trace=$(git config --bool "spatchCache.trace")
++trace=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_TRACE" --bool "spatchCache.trace")
+if test "$trace" != "true"
+then
+ trace=
@@ contrib/coccinelle/spatchcache (new)
+ trace=true
+fi
+
-+cacheWhenStderr=$(git config --bool "spatchCache.cacheWhenStderr")
++cacheWhenStderr=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_CACHEWHENSTDERR" --bool "spatchCache.cacheWhenStderr")
+if test "$cacheWhenStderr" != "true"
+then
+ cacheWhenStderr=
@@ contrib/coccinelle/spatchcache (new)
+ echo "$@" >&2
+}
+
-+spatch=$(git config --path "spatchCache.spatch" || :)
++spatch=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_SPATCH" --path "spatchCache.spatch")
+if test -n "$spatch"
+then
+ if test -n "$debug"
@@ contrib/coccinelle/spatchcache (new)
+fi
+
+dependFormat='$dirname/.depend/${basename%.c}.o.d'
-+dependFormatCfg=$(git config "spatchCache.dependFormat" || :)
++dependFormatCfg=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_DEPENDFORMAT" "spatchCache.dependFormat")
+if test -n "$dependFormatCfg"
+then
+ dependFormat="$dependFormatCfg"
+fi
+
-+set=$(git config spatchCache.setCmd || :)
-+get=$(git config spatchCache.getCmd || :)
++set=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_SETCMD" "spatchCache.setCmd")
++get=$(env_or_config "$GIT_CONTRIB_SPATCHCACHE_GETCMD" "spatchCache.getCmd")
+
+## Parse spatch()-like command-line for caching info
+arg_sp=-- 2.38.0.1251.g3eefdfb5e7a