From 960154b9c17afb276e12d0bec83513f3e46de565 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 6 May 2019 19:43:34 -0400 Subject: [PATCH 1/2] coccicheck: optionally batch spatch invocations In our "make coccicheck" rule, we currently feed each source file to its own individual invocation of spatch. This has a few downsides: - it repeats any overhead spatch has for starting up and reading the patch file - any included header files may get processed from multiple invocations. This is slow (we see the same header files multiple times) and may produce a resulting patch with repeated hunks (which cannot be applied without further cleanup) Ideally we'd just invoke a single instance of spatch per rule-file and feed it all source files. But spatch can be rather memory hungry when run in this way. I measured the peak RSS going from ~90MB for a single file to ~1900MB for all files. Multiplied by multiple rule files being processed at the same time (for "make -j"), this can make things slower or even cause them to fail (e.g., this is reported to happen on our Travis builds). Instead, let's provide a tunable knob. We'll leave the default at "1", but it can be cranked up to "999" for maximum CPU/memory tradeoff, or people can find points in between that serve their particular machines. Here are a few numbers running a single rule via: SIZES='1 4 16 999' RULE=contrib/coccinelle/object_id.cocci for i in $SIZES; do make clean /usr/bin/time -o $i.out --format='%e | %U | %S | %M' \ make $RULE.patch SPATCH_BATCH_SIZE=$i done for i in $SIZES; do printf '%4d | %s\n' $i "$(cat $i.out)" done which yields: 1 | 97.73 | 93.38 | 4.33 | 100128 4 | 52.80 | 51.14 | 1.69 | 135204 16 | 35.82 | 35.09 | 0.76 | 284124 999 | 23.30 | 23.13 | 0.20 | 1903852 The implementation is done with xargs, which should be widely available; it's in POSIX, we rely on it already in the test suite. And "coccicheck" is really a developer-only tool anyway, so it's not a big deal if obscure systems can't run it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 9f1b6e8926..daba958b8f 100644 --- a/Makefile +++ b/Makefile @@ -1174,8 +1174,10 @@ PTHREAD_CFLAGS = SPARSE_FLAGS ?= SP_EXTRA_FLAGS = -# For the 'coccicheck' target +# For the 'coccicheck' target; setting SPATCH_BATCH_SIZE higher will +# usually result in less CPU usage at the cost of higher peak memory. SPATCH_FLAGS = --all-includes --patch . +SPATCH_BATCH_SIZE = 1 include config.mak.uname -include config.mak.autogen @@ -2790,12 +2792,9 @@ endif %.cocci.patch: %.cocci $(COCCI_SOURCES) @echo ' ' SPATCH $<; \ - ret=0; \ - for f in $(COCCI_SOURCES); do \ - $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \ - { ret=$$?; break; }; \ - done >$@+ 2>$@.log; \ - if test $$ret != 0; \ + if ! echo $(COCCI_SOURCES) | xargs -n $(SPATCH_BATCH_SIZE) \ + $(SPATCH) --sp-file $< $(SPATCH_FLAGS) \ + >$@+ 2>$@.log; \ then \ cat $@.log; \ exit 1; \ From bcb4edf7af7f10878dd75ccfc3fc0f7596f2d658 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 8 May 2019 03:07:54 -0400 Subject: [PATCH 2/2] coccicheck: make batch size of 0 mean "unlimited" If you have the memory to handle it, the ideal case is to run a single spatch invocation with all of the source files. But the only way to do so now is to pick an arbitrarily large batch size. Let's make "0" do this, which is a little friendlier (and doesn't otherwise have a useful meaning). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index daba958b8f..9cea614523 100644 --- a/Makefile +++ b/Makefile @@ -1176,6 +1176,7 @@ SP_EXTRA_FLAGS = # 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 @@ -2792,7 +2793,12 @@ endif %.cocci.patch: %.cocci $(COCCI_SOURCES) @echo ' ' SPATCH $<; \ - if ! echo $(COCCI_SOURCES) | xargs -n $(SPATCH_BATCH_SIZE) \ + 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 \