[PATCH] coccicheck: optionally batch spatch invocations

On Mon, May 06, 2019 at 04:34:09PM +0700, Duy Nguyen wrote:

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.

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@peff.net>
---
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(-)