Re: [PATCH] coccicheck: optionally batch spatch invocations

From: Duy Nguyen <pclouds@gmail.com>
To: Jeff King <peff@peff.net>
Cc: "Jacob Keller" <jacob.keller@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Denton Liu" <liu.denton@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	"Thomas Gummerer" <t.gummerer@gmail.com>
Subject: Re: [PATCH] coccicheck: optionally batch spatch invocations
Date: Tue, 7 May 2019 17:20:41 +0700	[thread overview]
Message-ID: <CACsJy8BFxN9oL-ag2DH+mhySkOAXOzP1Q0wnb4bEHWTwgyGtfg@mail.gmail.com> (raw)
In-Reply-To: <20190506234334.GA13296@sigill.intra.peff.net>

On Tue, May 7, 2019 at 6:43 AM Jeff King <peff@peff.net> wrote:
>
> 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'm using 1.0.6

> 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

I think I'm going to settle around 32 which has peak at 2GB for me and
I can still make use of 4 hyperthread cores.

It's weird for me though that no SIZES=1 is actually the fastest. The
larger SPATCH_BATCH_SIZE is, the slower:

> ~/w/git/temp $ rm contrib/coccinelle/commit.cocci.patch
> ~/w/git/temp $ time make contrib/coccinelle/commit.cocci.patch
     SPATCH contrib/coccinelle/commit.cocci

real    1m20,242s
user    1m15,863s
sys     0m4,376s

> ~/w/git/temp $ rm contrib/coccinelle/commit.cocci.patch
> ~/w/git/temp $ time make contrib/coccinelle/commit.cocci.patch SPATCH_BATCH_SIZE=16
     SPATCH contrib/coccinelle/commit.cocci

real    3m3,131s
user    2m59,345s
sys     0m3,819s

> ~/w/git/temp $ rm contrib/coccinelle/commit.cocci.patch
> ~/w/git/temp $ time make contrib/coccinelle/commit.cocci.patch SPATCH_BATCH_SIZE=32
     SPATCH contrib/coccinelle/commit.cocci

real    5m10,966s
user    5m5,434s
sys     0m5,500s

But that might be just cocci or environmental problem. Since you and
Jacob both see good speedup, this patch is good.
-- 
Duy