Re: [PATCH v2 0/4] Makefile/coccicheck: fix bugs and speed it up

From: René Scharfe. <l.s.r@web.de>
Date: Mar 5, 2021, 19:10 UTC
Message-ID: <3aac381e-2ce9-e35e-498c-9c26df235aed@web.de>
In-Reply-To: <20210305170724.23859-1-avarab@gmail.com>

Am 05.03.21 um 18:07 schrieb Ævar Arnfjörð Bjarmason:

Show 18 quoted lines

> Addresses feedback on v1:
>
>  - The removal of the "cat $@.log" is gone.
>  - Split up into N changes.
>  - Explained why 960154b9c17 (coccicheck: optionally batch spatch
>    invocations, 2019-05-06) broke things and produced duplicate hunks
>    in 2/4: tl;dr: spatch does its own locking etc., xargs -n trips it
>    up.
>  - Set number of batch processes to 8, as suggested by Jeff King.
>
> Ævar Arnfjörð Bjarmason (4):
>   Makefile/coccicheck: add comment heading for all SPATCH flags
>   Makefile/coccicheck: speed up and fix bug with duplicate hunks
>   Makefile/coccicheck: allow for setting xargs concurrency
>   Makefile/coccicheck: set SPATCH_BATCH_SIZE to 8
>
>  Makefile | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)

This speeds up "make coccicheck" after "make clean" as advertized for me:

   before: 572.64s user 33.08s system 622% cpu 1:37.30  total
   after:  195.40s user  9.97s system 629% cpu   32.612 total

However, it also misses several conversions that coccicheck generated without this series for the example added by the patch at the bottom. Here's the difference of diffstats (< before, > after):

   $ diff <(diffstat b | sort) <(diffstat a | sort) | grep '^[<>]' | sort -k2 -k1
   >  27 files changed, 55 insertions(+), 57 deletions(-)
   <  36 files changed, 70 insertions(+), 71 deletions(-)
   <  attr.c                   |    2 +-
   >  blame.c                  |   15 +++++++--------
   <  blame.c                  |   17 ++++++++---------
   <  bloom.c                  |    2 +-
   <  cache-tree.c             |    2 +-
   >  combine-diff.c           |   18 +++++++++---------
   <  combine-diff.c           |   20 ++++++++++----------
   <  decorate.c               |    2 +-
   <  diffcore-rename.c        |    2 +-
   >  diffcore-rename.c        |    3 +--
   <  ewah/bitmap.c            |    2 +-
   <  hashmap.c                |    2 +-
   >  midx.c                   |    4 ++--
   <  midx.c                   |    8 ++++----
   <  pack-objects.c           |    2 +-
   <  pathspec.c               |    2 +-
   >  read-cache.c             |    2 +-
   <  read-cache.c             |    4 ++--
   >  ref-filter.c             |    2 +-
   <  ref-filter.c             |    4 ++--
   <  remote.c                 |    2 +-

That's with the current spatch version 1.1.0-00072-g3dc5d027.

diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci
index 46b8d2ee11..1f26da007a 100644
--- a/contrib/coccinelle/array.cocci
+++ b/contrib/coccinelle/array.cocci
@@ -88,3 +88,16 @@ expression n;
 @@
 - ptr = xmalloc((n) * sizeof(T));
 + ALLOC_ARRAY(ptr, n);
+
+@@
+type T;
+T *ptr;
+expression n != 1;
+@@
+(
+- ptr = xcalloc(n, \( sizeof(*ptr) \| sizeof(T) \));
++ CALLOC_ARRAY(ptr, n);
+|
+- ptr = xcalloc(\( sizeof(*ptr) \| sizeof(T) \), n);
++ CALLOC_ARRAY(ptr, n);
+)