git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] Makefile: fix bugs in coccicheck and speed it up

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 2, 2021, 20:51 UTC
Message-ID
<20210302205103.12230-1-avarab@gmail.com>
In-Reply-To
<20190506234334.GA13296@sigill.intra.peff.net>

I've often wondered why "make coccicheck" takes so long. This change speeds it up by ~2x and makes it use much less memory. Or a reduction of a max of around ~2GB per-process (under the old SPATCH_BATCH_SIZE=0) to around ~200MB.

Running the full "make coccicheck" now takes ~50 seconds with -j8 on my machine, v.s. ~2x of that before. I've got 64GB of memory on that machine, or it would be much slower.

Why has it been so slow? Because I think we've always been running it in entirely the wrong mode for what we wanted, and much of the previous fixing of this target has involved re-arranging the deck chairs on that particular Titanic.

What we really want to do with coccicheck is to do search/replacements in all our *.c and *.h files. This is now what we do, and we'll process a default of 64 files at a time.

What we were doing before was processing all our *.c files, and for each of those *.c files we'd recursively look around for includes and see if we needed to search/replace in those too.

That we did that dates back to [1] when we were only processing *.c files, and it was always very redundant. We'd e.g. visit the likes of strbuf.h lots of times since it's widely used as an include.

Then in the most recent attempt to optimize coccicheck in [2] this anti-pattern finally turned into a bug.

Namely: before this change, if your coccicheck rule applied to
e.g. making a change in strbuf.h itself we'd get *lots* of duplicate
hunks applying the exact same change, as concurrent spatch processes
invoked by xargs raced one another. In one instance I ended up with 27
copies of the same hunk in a strbuf.patch.

Setting SPATCH_BATCH_SIZE=0 and processing all the files in one giant batch mitigated this. I suspect the author of [2] either mostly ran in that mode, or didn't test on changes that impacted widely used header files.

So since we're going to want to process all our *.c and *.h let's just do that, and drop --all-includes for --no-includes. It's not spatch's job to find our sources, we're doing that. If someone is manually tweaking COCCI_SOURCES they can just tweak SPATCH_FLAGS too.

I'm entirely removing SPATCH_BATCH_SIZE. If you want to tweak it you can tweak SPATCH_XARGS_FLAGS to e.g. "-n 256", or "-P 4 -n 128". But in my testing it isn't worth it to tweak SPATCH_XARGS_FLAGS for a full "make coccicheck".

I'm also the whole "cat $@.log" introduced in [3]. Since we don't call this in a loop anymore (and xargs will early-exit) we can just rely on standard V=1 for debugging issues.

1. a9a884aea5 (coccicheck: use --all-includes by default, 2016-09-30)
2. 960154b9c1 (coccicheck: optionally batch spatch invocations,
   2019-05-06)
3. f5c2bc2b96 (Makefile: detect errors in running spatch, 2017-03-10)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/Makefile b/Makefile
index dd08b4ced0..2108df8913 100644
--- a/Makefile
+++ b/Makefile
@@ -1195,11 +1195,20 @@ PTHREAD_CFLAGS =
 SPARSE_FLAGS ?=
 SP_EXTRA_FLAGS = -Wno-universal-initializer
 
-# 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.
-SPATCH_FLAGS = --all-includes --patch .
-SPATCH_BATCH_SIZE = 1
+SPATCH_FLAGS = --no-includes --patch .
+# For the 'coccicheck' target; Tweaking SPATCH_XARGS_FLAGS is
+# generally not neccesary with a top-level -jN.
+#
+# To get concurrency when targeting a single
+# contrib/coccinelle/%.patch use e.g. "-P" if your xargs(1) supports
+# it:
+#
+#    make contrib/coccinelle/strbuf.cocci.patch SPATCH_XARGS_FLAGS="-P 8 -n 64"
+#
+# Or a combination of the two:
+#
+#    make -j4 coccicheck SPATCH_XARGS_FLAGS="-P 2 -n 64"
+SPATCH_XARGS_FLAGS =
 
 include config.mak.uname
 -include config.mak.autogen
@@ -2852,24 +2861,18 @@ check: config-list.h command-list.h
 		exit 1; \
 	fi
 
-FOUND_C_SOURCES = $(filter %.c,$(shell $(FIND_SOURCE_FILES)))
+FOUND_C_SOURCES = $(filter %.c %.h,$(shell $(FIND_SOURCE_FILES)))
 COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
 
 %.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) --sp-file $< $(SPATCH_FLAGS) \
-		>$@+ 2>$@.log; \
-	then \
-		cat $@.log; \
-		exit 1; \
-	fi; \
-	mv $@+ $@; \
+	$(RM) $@+ $@.log && \
+	echo $(COCCI_SOURCES) | \
+		xargs \
+			-n 32 $(SPATCH_XARGS_FLAGS) \
+			$(SPATCH) --sp-file $< $(SPATCH_FLAGS) \
+		>>$@+ 2>>$@.log && \
+	mv $@+ $@ && \
 	if test -s $@; \
 	then \
 		echo '    ' SPATCH result: $@; \
-- 
2.31.0.rc0.126.g04f22c5b82
Previous: SZEDER GáborNext: Ævar Arnfjörð Bjarmason
Message 46 of 103 in “*.h: remove extern from function declarations”
  1. *.h: remove extern from function declarationsDenton Liu, Apr 12, 2019
  2. Jeff KingApr 13, 2019
  3. Junio C HamanoApr 13, 2019
  4. 0/3 remove extern from function declarationsDenton Liu, Apr 15, 2019
  5. 2/3 *.[ch]: remove extern from function declarations using sedDenton Liu, Apr 15, 2019
  6. 3/3 cocci: prevent extern function declarationsDenton Liu, Apr 15, 2019
  7. 1/3 *.[ch]: remove extern from function declarations using spatchDenton Liu, Apr 15, 2019
  8. Thomas GummererApr 15, 2019
  9. 0/4 remove extern from function declarationsDenton Liu, Apr 17, 2019
  10. 2/4 *.[ch]: remove extern from function declarations using sedDenton Liu, Apr 17, 2019
  11. 1/4 *.[ch]: remove extern from function declarations using spatchDenton Liu, Apr 17, 2019
  12. 4/4 cocci: prevent extern function declarationsDenton Liu, Apr 17, 2019
  13. 3/4 *.[ch]: manually align parameter listsDenton Liu, Apr 17, 2019
  14. cache.h: fix mismerge of 'dl/no-extern-in-func-decl'Denton Liu, Apr 22, 2019
  15. Junio C HamanoApr 22, 2019
  16. Junio C HamanoApr 22, 2019
  17. Jeff KingApr 22, 2019
  18. SZEDER GáborApr 25, 2019
  19. Denton LiuApr 25, 2019
  20. Johannes SchindelinApr 30, 2019
  21. Denton LiuMay 1, 2019
  22. Jeff KingMay 1, 2019
  23. SZEDER GáborMay 2, 2019
  24. Johannes SchindelinMay 3, 2019
  25. SZEDER GáborMay 3, 2019
  26. SZEDER GáborMay 3, 2019
  27. Jeff KingMay 3, 2019
  28. SZEDER GáborMay 3, 2019
  29. Junio C HamanoMay 5, 2019
  30. Jacob KellerMay 5, 2019
  31. Jacob KellerMay 5, 2019
  32. coccicheck: optionally process every source file at onceJeff King, May 6, 2019
  33. Duy NguyenMay 6, 2019
  34. coccicheck: optionally batch spatch invocationsJeff King, May 6, 2019
  35. Jacob KellerMay 7, 2019
  36. Jeff KingMay 7, 2019
  37. Junio C HamanoMay 7, 2019
  38. Jeff KingMay 7, 2019
  39. Jacob KellerMay 7, 2019
  40. Junio C HamanoMay 7, 2019
  41. Jeff KingMay 8, 2019
  42. Denton LiuMay 8, 2019
  43. Jeff KingMay 8, 2019
  44. Duy NguyenMay 7, 2019
  45. SZEDER GáborMay 7, 2019
  46. Makefile: fix bugs in coccicheck and speed it upÆvar Arnfjörð Bjarmason, Mar 2, 2021
  47. Ævar Arnfjörð BjarmasonMar 3, 2021
  48. Denton LiuMar 3, 2021
  49. Junio C HamanoMar 4, 2021
  50. Ævar Arnfjörð BjarmasonMar 5, 2021
  51. Jeff KingMar 5, 2021
  52. Ævar Arnfjörð BjarmasonMar 5, 2021
  53. Jeff KingMar 6, 2021
  54. 1/4 Makefile/coccicheck: add comment heading for all SPATCH flagsÆvar Arnfjörð Bjarmason, Mar 5, 2021
  55. 0/4 Makefile/coccicheck: fix bugs and speed it upÆvar Arnfjörð Bjarmason, Mar 5, 2021
  56. René Scharfe.Mar 5, 2021
  57. Ævar Arnfjörð BjarmasonMar 6, 2021
  58. René Scharfe.Mar 6, 2021
  59. Ævar Arnfjörð BjarmasonMar 6, 2021
  60. René Scharfe.Mar 6, 2021
  61. 3/4 Makefile/coccicheck: allow for setting xargs concurrencyÆvar Arnfjörð Bjarmason, Mar 5, 2021
  62. Jeff KingMar 6, 2021
  63. 4/4 Makefile/coccicheck: set SPATCH_BATCH_SIZE to 8Ævar Arnfjörð Bjarmason, Mar 5, 2021
  64. 5/4 Makefile/coccicheck: use --include-headers-for-typesÆvar Arnfjörð Bjarmason, Mar 6, 2021
  65. SZEDER GáborMar 18, 2021
  66. Ævar Arnfjörð BjarmasonMar 19, 2021
  67. 0/4 Makefile/coccicheck: fix bugs and speed it upÆvar Arnfjörð Bjarmason, Mar 22, 2021
  68. 1/4 Makefile/coccicheck: add comment heading for all SPATCH flagsÆvar Arnfjörð Bjarmason, Mar 22, 2021
  69. René Scharfe.Mar 22, 2021
  70. 4/4 Makefile/coccicheck: set SPATCH_BATCH_SIZE to 8Ævar Arnfjörð Bjarmason, Mar 22, 2021
  71. René Scharfe.Mar 22, 2021
  72. Jeff KingMar 24, 2021
  73. 2/4 Makefile/coccicheck: speed up and fix bug with duplicate hunksÆvar Arnfjörð Bjarmason, Mar 22, 2021
  74. René Scharfe.Mar 22, 2021
  75. Jeff KingMar 24, 2021
  76. Junio C HamanoMar 22, 2021
  77. 3/4 Makefile/coccicheck: allow for setting xargs concurrencyÆvar Arnfjörð Bjarmason, Mar 22, 2021
  78. Jeff KingMar 24, 2021
  79. Ævar Arnfjörð BjarmasonMar 25, 2021
  80. Jeff KingMar 26, 2021
  81. Junio C HamanoMar 27, 2021
  82. Ævar Arnfjörð BjarmasonMar 27, 2021
  83. 2/4 Makefile/coccicheck: speed up and fix bug with duplicate hunksÆvar Arnfjörð Bjarmason, Mar 5, 2021
  84. Jeff KingMar 6, 2021
  85. Ævar Arnfjörð BjarmasonMar 6, 2021
  86. Denton LiuMay 3, 2019
  87. 0/4 remove extern from function declarationsDenton Liu, Apr 23, 2019
  88. 2/4 *.[ch]: remove extern from function declarations using sedDenton Liu, Apr 23, 2019
  89. Junio C HamanoApr 24, 2019
  90. Denton LiuApr 25, 2019
  91. 3/4 *.[ch]: manually align parameter listsDenton Liu, Apr 23, 2019
  92. 4/4 cocci: prevent extern function declarationsDenton Liu, Apr 23, 2019
  93. 1/4 *.[ch]: remove extern from function declarations using spatchDenton Liu, Apr 23, 2019
  94. 0/3 *** SUBJECT HERE ***Denton Liu, Apr 29, 2019
  95. 2/3 *.[ch]: remove extern from function declarations using sedDenton Liu, Apr 29, 2019
  96. 1/3 *.[ch]: remove extern from function declarations using spatchDenton Liu, Apr 29, 2019
  97. 3/3 *.[ch]: manually align parameter listsDenton Liu, Apr 29, 2019
  98. Denton LiuApr 29, 2019
  99. Ævar Arnfjörð BjarmasonMay 6, 2019
  100. Denton LiuMay 6, 2019
  101. René Scharfe.Mar 6, 2021
  102. René Scharfe.Mar 6, 2021
  103. René Scharfe.Mar 13, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.