On Mon, May 06, 2019 at 04:34:09PM +0700, Duy Nguyen wrote:
> > However, it comes at a cost. The RSS of each spatch process goes from
> > ~50MB to ~1500MB (and peak memory usage may be even higher if make runs
>
> 1.5G should be fine. Trying...
>
> Even with no -j, my htop's RES column goes up 6GB and put my laptop in
> "swap every bit of memory out, including the bits handling the screen"
> mode :( I don't think it was even the peak.
Interesting if you have a different version of spatch. I'm using 1.0.4
from Debian unstable.
I had just been eyeballing the values in "top" before, but I actually
measured more carefully. My peak was actually ~1900MB.
> It's probably a bit too much to ask, but is it possible to handle N
> files at a time (instead of all files), which consumes less memory and
> runs a bit slower, but still better than the default mode? I can see
> it already gets tricky doing complicated stuff in Makefile so "no" is
> perfectly ok.
I almost did this initially but I feared that nobody would actually use
it. :) So given at least one person who wants it, I took a look. If we
rely on xargs, then it is really not too bad (and is in fact shorter
than the current code). I also wrote up a pure-shell version, but it's
rather verbose even after taking some shortcuts with whitespace
splitting.
So here's what I think we should apply:
-- >8 --
Subject: [PATCH] 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 <peff@xxxxxxxx>
---
I left the default at 1 for safety. Probably 4 or 16 would be an OK
default, but I don't have any interest in figuring out exactly what
Travis or some hypothetical average machine can handle. I'll be setting
mine to 999. ;)
Making "0" work as "unlimited" might be nice, but xargs doesn't support
that and I didn't want to make the recipe any more unreadable than it
already is.
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; \
--
2.21.0.1314.g224b191707
