archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <>
To: "SZEDER Gábor" <>
Cc: Junio C Hamano <>, Jeff King <>,
	Derrick Stolee <>,
	Derrick Stolee via GitGitGadget <>,
Subject: Re: [PATCH] ci: update 'static-analysis' to Ubuntu 22.04
Date: Wed, 31 Aug 2022 14:13:51 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Wed, Aug 31 2022, SZEDER Gábor wrote:

> On Fri, Aug 26, 2022 at 09:46:54AM -0700, Junio C Hamano wrote:
>> Jeff King <> writes:
>> >> But the fix here isn't to delete unused.cocci, but to hold off on the
>> >> UNUSEwork D() patches until we figure out how to make coccinelle jive with
>> >> them.
>> >
>> > Yeah, my general skepticism and disappointment above notwithstanding,
>> > this seems like the best path forward from here. I tried a few other
>> > tricks (like --macro-file and --iso-file), but if its parser chokes, I
>> > don't think there's much we can do about it. Even if we wrote a patch to
>> > coccinelle itself (and I have no interest in doing that myself), it
>> > would take a while to become available.
>> If it is just a single unused.cocci, I would actually think removing
>> it would be a much better path forward.  UNUSED() that renames to
>> help folks without checking compilers would help noticing bad code
>> much earlier than unused.cocci many contributors are not running
>> themselves anyway.
> Here is another reason for the removal of 'unused.cocci': it's very
> costly to apply that semantic patch to the whole code base.
>   make SPATCH_BATCH_SIZE=32 contrib/coccinelle/unused.cocci.patch
> takes 440s on my machine, whereas the second slowest 'object_id.cocci'
> takes only 56s [1].  Applying 'unused.cocci' to some of our source files
> individually takes well over a minute:
>   $ time spatch --all-includes --sp-file contrib/coccinelle/unused.cocci builtin/log.c
>   warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
>   warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
>   HANDLING: builtin/log.c
>   Note: processing took    83.1s: builtin/log.c
>   real	1m23.083s
>   user	1m22.983s

If you remove the "done:" line in cmd_format_patch() buiiltin/log.c runs
in ~200ms instead of ~40s for me. Perhaps we should be discussing
removing or refactoring that one line of code instead? :)

Removing coccinelle rules because we're seeing slowness somewhere seems
particularly short-sighted to me.

Maybe we do run into intractable problems somewhere with it being slow,
and we'd also like to cater to more "interactive" use.

But we shouldn't do that by removing rules until we get below some
runtime limit, but rather by creating a "batch" category or something
(just like we have "pending") now.

Or, just actually look into why it's slow and fix those issues and/or
report them upstream.

There's nothing in unused.cocci that we either aren't running into
elsewhere, or wouldn't run into if we had 10x the coccinelle rules we
have now (which I think would be a good direction, we should rely on it
more heavily).

I've found that being able to have a ccache-like tool for "spatch"[1]
solved almost all of the practical performance concerns I had with
it. I.e. I can just run things in a batch, and usually any interactive
use will hit things already in cache.

To the extent it doesn't it's usually some pathological issue in spatch.

>   sys	0m0.033s
>   $ time spatch --all-includes --sp-file contrib/coccinelle/unused.cocci builtin/rebase.c 
>   warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
>   warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
>   HANDLING: builtin/rebase.c
>   Note: processing took    83.2s: builtin/rebase.c
>   real	1m23.223s
>   user	1m23.156s
>   sys	0m0.017s

I didn't look at this one, but I assume it's some similar (and probably
easily fixed) pathological issue.


  reply	other threads:[~2022-08-31 12:29 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-23 17:28 [PATCH] ci: update 'static-analysis' to Ubuntu 22.04 Derrick Stolee via GitGitGadget
2022-08-24 14:40 ` Johannes Schindelin
2022-08-24 19:59   ` Junio C Hamano
2022-08-24 23:43 ` Junio C Hamano
2022-08-25  0:30   ` Derrick Stolee
2022-08-25  4:43     ` Junio C Hamano
2022-08-25 10:47       ` Ævar Arnfjörð Bjarmason
2022-08-25 16:08         ` Junio C Hamano
2022-08-25 17:09           ` [PATCH 0/2] git-compat-util.h: change UNUSED(var) to UNUSED Ævar Arnfjörð Bjarmason
2022-08-25 17:09             ` [PATCH 1/2] git-compat-util.h: use "UNUSED", not "UNUSED(var)" Ævar Arnfjörð Bjarmason
2022-08-25 17:09             ` [PATCH 2/2] git-compat-util.h: use "deprecated" for UNUSED variables Ævar Arnfjörð Bjarmason
2022-08-26  7:52             ` [PATCH 0/2] git-compat-util.h: change UNUSED(var) to UNUSED Jeff King
2022-08-26  7:48         ` [PATCH] ci: update 'static-analysis' to Ubuntu 22.04 Jeff King
2022-08-26 16:46           ` Junio C Hamano
2022-08-27 12:58             ` Jeff King
2022-08-29  5:56               ` Junio C Hamano
2022-08-29 10:29             ` Ævar Arnfjörð Bjarmason
2022-08-31 15:12               ` Jeff King
2022-08-31  8:44             ` SZEDER Gábor
2022-08-31 12:13               ` Ævar Arnfjörð Bjarmason [this message]
2022-08-31 15:24                 ` Jeff King
2022-08-31 19:19                   ` Junio C Hamano
2022-08-31 18:05                 ` SZEDER Gábor
2022-08-31 19:29                   ` Ævar Arnfjörð Bjarmason
2022-08-25 14:57 ` Ævar Arnfjörð Bjarmason
2022-08-25 16:17   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).