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@xxxxxxxxxxxxxxxxxxx>) on the
cocci@xxxxxxxx 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@xxxxxxxxx>
---
Makefile | 40 ++++++++++++++++++++++++++++++++---
contrib/coccinelle/.gitignore | 1 +
contrib/coccinelle/README | 13 ++++++++++++
shared.mak | 1 +
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 531077031d8..0d65c09ad5a 100644
--- a/Makefile
+++ b/Makefile
@@ -1311,6 +1311,25 @@ SPATCH_TEST_FLAGS =
# COMPUTE_HEADER_DEPENDENCIES=no this will be unset too.
SPATCH_USE_O_DEPENDENCIES = YesPlease
+# Set SPATCH_CONCAT_COCCI to concatenate the contrib/cocci/*.cocci
+# files into a single contrib/cocci/ALL.cocci before running
+# "coccicheck".
+#
+# Pros:
+#
+# - Speeds up a one-shot run of "make coccicheck", as we won't have to
+# parse *.[ch] files N times for the N *.cocci rules
+#
+# Cons:
+#
+# - Will make incremental development of *.cocci slower, as
+# e.g. changing strbuf.cocci will re-run all *.cocci.
+#
+# - Makes error and performance analysis harder, as rules will be
+# applied from a monolithic ALL.cocci, rather than
+# e.g. strbuf.cocci.
+SPATCH_CONCAT_COCCI = YesPlease
+
# Rebuild 'coccicheck' if $(SPATCH), its flags etc. change
TRACK_SPATCH_DEFINES =
TRACK_SPATCH_DEFINES += $(SPATCH)
@@ -3159,9 +3178,10 @@ check: $(GENERATED_H)
exit 1; \
fi
+COCCI_GEN_ALL = contrib/coccinelle/ALL.cocci
COCCI_GLOB = $(wildcard contrib/coccinelle/*.cocci)
-COCCI_RULES = $(COCCI_GLOB)
-COCCI_NAMES = $(COCCI_RULES:contrib/coccinelle/%.cocci=%)
+COCCI_NAMES = $(sort ALL $(COCCI_GLOB:contrib/coccinelle/%.cocci=%))
+COCCI_RULES = $(filter-out $(COCCI_GEN_ALL),$(COCCI_GLOB))
COCCICHECK_PENDING = $(filter %.pending.cocci,$(COCCI_RULES))
COCCICHECK_PATCHES_PENDING = $(COCCICHECK_PENDING:%=%.patch)
@@ -3175,6 +3195,7 @@ COCCI_RULES_GLOB += cocci%
COCCI_RULES_GLOB += .build/contrib/coccinelle/%
COCCI_RULES_GLOB += $(COCCICHECK_PATCHES)
COCCI_RULES_GLOB += $(COCCICHEC_PATCHES_PENDING)
+COCCI_RULES_GLOB += $(COCCI_GEN_ALL)
COCCI_GOALS = $(filter $(COCCI_RULES_GLOB),$(MAKECMDGOALS))
COCCICHECK_PATCHES = $(COCCICHECK:%=%.patch)
@@ -3186,6 +3207,10 @@ COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
$(call mkdir_p_parent_template)
$(QUIET_GEN) >$@
+$(COCCI_GEN_ALL): $(COCCICHECK)
+ $(call mkdir_p_parent_template)
+ $(QUIET_SPATCH_CAT)cat $^ >$@
+
ifeq ($(COMPUTE_HEADER_DEPENDENCIES),no)
SPATCH_USE_O_DEPENDENCIES =
endif
@@ -3218,7 +3243,7 @@ $(foreach s,$(COCCI_SOURCES),$(call cocci-rule,$(c),$(s),$(s:%.c=%.o)))
endef
ifdef COCCI_GOALS
-$(eval $(foreach c,$(COCCI_RULES),$(call cocci-matrix,$(c))))
+$(eval $(foreach c,$(COCCI_RULES) $(COCCI_GEN_ALL),$(call cocci-matrix,$(c))))
endif
define spatch-rule
@@ -3239,7 +3264,11 @@ COCCI_TEST_RES_GEN = $(addprefix .build/,$(COCCI_TEST_RES))
$(COCCI_TEST_RES_GEN): GIT-SPATCH-DEFINES
$(COCCI_TEST_RES_GEN): .build/%.res : %.c
$(COCCI_TEST_RES_GEN): .build/%.res : %.res
+ifdef SPATCH_CONCAT_COCCI
+$(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : $(COCCI_GEN_ALL)
+else
$(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib/coccinelle/%.cocci
+endif
$(call mkdir_p_parent_template)
$(QUIET_SPATCH_TEST)$(SPATCH) $(SPATCH_TEST_FLAGS) \
--very-quiet --no-show-diff \
@@ -3252,7 +3281,11 @@ $(COCCI_TEST_RES_GEN): .build/contrib/coccinelle/tests/%.res : contrib/coccinell
coccicheck-test: $(COCCI_TEST_RES_GEN)
coccicheck: coccicheck-test
+ifdef SPATCH_CONCAT_COCCI
+coccicheck: contrib/coccinelle/ALL.cocci.patch
+else
coccicheck: $(COCCICHECK_PATCHES)
+endif
# See contrib/coccinelle/README
coccicheck-pending: coccicheck-test
@@ -3527,6 +3560,7 @@ cocciclean:
$(RM) -r .build/contrib/coccinelle
$(RM) $(COCCICHECK_PATCHES)
$(RM) $(COCCICHECK_PATCHES_PENDING)
+ $(RM) $(COCCI_GEN_ALL)
clean: profile-clean coverage-clean cocciclean
$(RM) -r .build
diff --git a/contrib/coccinelle/.gitignore b/contrib/coccinelle/.gitignore
index d3f29646dc3..2709d98eb91 100644
--- a/contrib/coccinelle/.gitignore
+++ b/contrib/coccinelle/.gitignore
@@ -1 +1,2 @@
+/ALL.cocci
*.patch*
diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README
index 09ea8298e19..09b72bfd4e7 100644
--- a/contrib/coccinelle/README
+++ b/contrib/coccinelle/README
@@ -57,3 +57,16 @@ 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).
diff --git a/shared.mak b/shared.mak
index 5ccd6889fcb..66dcf6768db 100644
--- a/shared.mak
+++ b/shared.mak
@@ -73,6 +73,7 @@ ifndef V
## Used in "Makefile": SPATCH
QUIET_SPATCH = @echo ' ' SPATCH $@;
QUIET_SPATCH_TEST = @echo ' ' SPATCH TEST $(@:.build/%=%);
+ QUIET_SPATCH_CAT = @echo ' ' SPATCH CAT $$^ \>$@;
## Used in "Makefile": SPATCH_*TMPL (quoted for use in "define"'s)
QUIET_SPATCH_CAT_TMPL = @echo ' ' SPATCH CAT $$$$^ \>$$@;
--
2.38.0.1251.g3eefdfb5e7a
