Re: [PATCH v2 7/9] cocci: make "coccicheck" rule incremental

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sep 1, 2022, 16:38 UTC
Message-ID: <20220901163859.GA122568@szeder.dev>
In-Reply-To: <patch-v2-7.9-120607b5da6-20220831T205130Z-avarab@gmail.com>

On Wed, Aug 31, 2022 at 10:57:54PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 8 quoted lines

> Optimize the very slow "coccicheck" target to take advantage of
> incremental rebuilding, and fix outstanding dependency problems with
> the existing rule.
> 
> The rule is now faster both on the initial run as we can make better
> use of GNU make's parallelism than the old ad-hoc combination of
> make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the
> "--jobs" argument to "spatch(1)".

On my machine a "from scratch" 'make -j4 coccicheck' without the inacceptably slow 'unused.cocci' takes 9m28s, with SPATCH_BATCH_SIZE=32 it takes 6m39s. If we invoke 'spatch' like in the patch below and let one 'spatch' invocation process all source files one by one (i.e. unlike the batched invocations) and using its '--jobs' option then it takes 4m56s.

This patch series is slower than any of the above, as it takes 10m3s.

Show 8 quoted lines

>  * Before this change running "make coccicheck" would by default end
>    up pegging just one CPU at the very end for a while, usually as
>    we'd finish whichever *.cocci rule was the most expensive.
> 
>    This could be mitigated by combining "make -jN" with
>    SPATCH_BATCH_SIZE, see 960154b9c17 (coccicheck: optionally batch
>    spatch invocations, 2019-05-06). But doing so required careful
>    juggling, as e.g. setting both to 4 would yield 16 workers.

As pointed out previously, this is not the case.

  ---  >8  ---

diff --git a/Makefile b/Makefile
index e8adeb09f1..b93b1b62e1 100644
--- a/Makefile
+++ b/Makefile
@@ -1290,11 +1290,9 @@ SP_EXTRA_FLAGS = -Wno-universal-initializer
 SANITIZE_LEAK =
 SANITIZE_ADDRESS =
 
-# For the 'coccicheck' target; setting SPATCH_BATCH_SIZE higher will
-# usually result in less CPU usage at the cost of higher peak memory.
-# Setting it to 0 will feed all files in a single spatch invocation.
+# For the 'coccicheck' target
 SPATCH_FLAGS = --all-includes
-SPATCH_BATCH_SIZE = 1
+SPATCH_JOBS = 1
 
 include config.mak.uname
 -include config.mak.autogen
@@ -3139,19 +3137,17 @@ COCCI_TEST_RES = $(wildcard contrib/coccinelle/tests/*.res)
 
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
 	$(QUIET_SPATCH) \
-	if test $(SPATCH_BATCH_SIZE) = 0; then \
-		limit=; \
-	else \
-		limit='-n $(SPATCH_BATCH_SIZE)'; \
-	fi; \
-	if ! echo $(COCCI_SOURCES) | xargs $$limit \
-		$(SPATCH) $(SPATCH_FLAGS) \
-		--sp-file $< --patch . \
+	filelist=$@.filelist; \
+	: Blank lines between filenames are necessary to put each file in separate group ; \
+	printf "%s\\n\\n" $(COCCI_SOURCES) >$$filelist && \
+	if ! $(SPATCH) $(SPATCH_FLAGS) --jobs $(SPATCH_JOBS) \
+		--sp-file $< --patch . --file-groups $$filelist \
 		>$@+ 2>$@.log; \
 	then \
 		cat $@.log; \
 		exit 1; \
 	fi; \
+	rm -f $$filelist; \
 	mv $@+ $@; \
 	if test -s $@; \
 	then \