git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v4 11/12] cocci: run against a generated ALL.cocci
Date: Wed, 26 Oct 2022 16:20:38 +0200	[thread overview]
Message-ID: <patch-v4-11.12-a848d09527f-20221026T141005Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v4-00.12-00000000000-20221026T141005Z-avarab@gmail.com>

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>
---
 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


  parent reply	other threads:[~2022-10-26 14:22 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-25 14:36 [PATCH 0/5] cocci: make "incremental" possible + a ccache-like tool Ævar Arnfjörð Bjarmason
2022-08-25 14:36 ` [PATCH 1/5] Makefile: add ability to TAB-complete cocci *.patch rules Ævar Arnfjörð Bjarmason
2022-08-25 14:36 ` [PATCH 2/5] Makefile: have "coccicheck" re-run if flags change Ævar Arnfjörð Bjarmason
2022-08-25 15:29   ` SZEDER Gábor
2022-08-25 14:36 ` [PATCH 3/5] cocci: make "coccicheck" rule incremental Ævar Arnfjörð Bjarmason
2022-08-25 19:44   ` SZEDER Gábor
2022-08-25 22:18     ` Ævar Arnfjörð Bjarmason
2022-08-26 10:43       ` SZEDER Gábor
2022-08-25 14:36 ` [PATCH 4/5] cocci: make incremental compilation even faster Ævar Arnfjörð Bjarmason
2022-08-25 14:36 ` [PATCH 5/5] spatchcache: add a ccache-alike for "spatch" Ævar Arnfjörð Bjarmason
2022-08-31 20:57 ` [PATCH v2 0/9] cocci: make "incremental" possible + a ccache-like tool Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 1/9] cocci rules: remove unused "F" metavariable from pending rule Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 2/9] Makefile: add ability to TAB-complete cocci *.patch rules Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 3/9] Makefile: have "coccicheck" re-run if flags change Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 4/9] Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 5/9] cocci: split off include-less "tests" from SPATCH_FLAGS Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 6/9] cocci: split off "--all-includes" " Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 7/9] cocci: make "coccicheck" rule incremental Ævar Arnfjörð Bjarmason
2022-09-01 16:38     ` SZEDER Gábor
2022-09-01 18:04       ` Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 8/9] cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES Ævar Arnfjörð Bjarmason
2022-08-31 20:57   ` [PATCH v2 9/9] spatchcache: add a ccache-alike for "spatch" Ævar Arnfjörð Bjarmason
2022-10-14 15:31   ` [PATCH v3 00/11] cocci: make "incremental" possible + a ccache-like tool Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 01/11] Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T) Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 02/11] cocci rules: remove unused "F" metavariable from pending rule Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 03/11] Makefile: add ability to TAB-complete cocci *.patch rules Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 04/11] Makefile: have "coccicheck" re-run if flags change Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 05/11] Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading Ævar Arnfjörð Bjarmason
2022-10-14 20:39       ` Taylor Blau
2022-10-14 15:31     ` [PATCH v3 06/11] cocci: split off include-less "tests" from SPATCH_FLAGS Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 07/11] cocci: split off "--all-includes" " Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 08/11] cocci: make "coccicheck" rule incremental Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 09/11] cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 10/11] cocci: run against a generated ALL.cocci Ævar Arnfjörð Bjarmason
2022-10-14 15:31     ` [PATCH v3 11/11] spatchcache: add a ccache-alike for "spatch" Ævar Arnfjörð Bjarmason
2022-10-17 17:50     ` [PATCH v3 00/11] cocci: make "incremental" possible + a ccache-like tool Jeff King
2022-10-17 18:36       ` Ævar Arnfjörð Bjarmason
2022-10-17 19:08         ` Junio C Hamano
2022-10-17 19:18         ` Jeff King
2022-10-26 14:20     ` [PATCH v4 00/12] " Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 01/12] Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T) Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 02/12] cocci rules: remove unused "F" metavariable from pending rule Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 03/12] Makefile: add ability to TAB-complete cocci *.patch rules Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 04/12] Makefile: have "coccicheck" re-run if flags change Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 05/12] Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 06/12] cocci: split off include-less "tests" from SPATCH_FLAGS Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 07/12] cocci: split off "--all-includes" " Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 08/12] cocci: make "coccicheck" rule incremental Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 09/12] cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` [PATCH v4 10/12] cocci rules: remove <id>'s from rules that don't need them Ævar Arnfjörð Bjarmason
2022-10-26 14:20       ` Ævar Arnfjörð Bjarmason [this message]
2022-10-28 12:58         ` [PATCH v4 11/12] cocci: run against a generated ALL.cocci SZEDER Gábor
2022-10-26 14:20       ` [PATCH v4 12/12] spatchcache: add a ccache-alike for "spatch" Ævar Arnfjörð Bjarmason
2022-11-01 22:35       ` [PATCH v5 00/13] cocci: make "incremental" possible + a ccache-like tool Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 01/13] Makefile + shared.mak: rename and indent $(QUIET_SPATCH_T) Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 02/13] cocci rules: remove unused "F" metavariable from pending rule Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 03/13] Makefile: add ability to TAB-complete cocci *.patch rules Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 04/13] Makefile: have "coccicheck" re-run if flags change Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 05/13] Makefile: split off SPATCH_BATCH_SIZE comment from "cocci" heading Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 06/13] cocci: split off include-less "tests" from SPATCH_FLAGS Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 07/13] cocci: split off "--all-includes" " Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 08/13] cocci: make "coccicheck" rule incremental Ævar Arnfjörð Bjarmason
2022-11-09 14:57           ` SZEDER Gábor
2022-11-01 22:35         ` [PATCH v5 09/13] cocci: optimistically use COMPUTE_HEADER_DEPENDENCIES Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 10/13] Makefile: copy contrib/coccinelle/*.cocci to build/ Ævar Arnfjörð Bjarmason
2022-11-09 15:05           ` SZEDER Gábor
2022-11-09 15:42             ` Ævar Arnfjörð Bjarmason
2022-11-10 16:14               ` [PATCH] Makefile: don't create a ".build/.build/" for cocci, fix output Ævar Arnfjörð Bjarmason
2022-11-11 22:22                 ` Taylor Blau
2022-11-01 22:35         ` [PATCH v5 11/13] cocci rules: remove <id>'s from rules that don't need them Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 12/13] cocci: run against a generated ALL.cocci Ævar Arnfjörð Bjarmason
2022-11-01 22:35         ` [PATCH v5 13/13] spatchcache: add a ccache-alike for "spatch" Ævar Arnfjörð Bjarmason

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=patch-v4-11.12-a848d09527f-20221026T141005Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.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).