Subject: Re: [PATCH v2 5/4] Makefile/coccicheck: use --include-headers-for-types

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 19 Mar 2021 11:32:39 +0100

On Thu, Mar 18 2021, SZEDER Gábor wrote:

> On Sat, Mar 06, 2021 at 08:25:25PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> Change the recently added --no-includes out for --all-includes
>> --include-headers-for-types.
>> 
>> When I moved from --all-includes to --no-includes I missed that rules
>> that use "type T" such as the one added dbc540c7a58 (add QSORT,
>> 2016-09-29) won't work as intended. See [1] for a report of that.
>> 
>> So let's move back to --all-includes to potentially scour our includes
>> for definitions, but supply --include-headers-for-types so we won't
>> racily emit duplicate hunks. See [2] for more discussion about that
>> issue.
>> 
>> This undoes most of the optimization we gained with --no-includes,
>> using a very basic benchmark script [3] I get these results:
>> 
>>  * ~1m50s: --all-includes (before my recent patches)
>>    38 files changed, 74 insertions(+), 78 deletions(-)
>>  * ~55s:   --no-includes
>>    27 files changed, 55 insertions(+), 57 deletions(-)
>>  * ~1m35s: --all-includes --include-headers-for-types
>>    38 files changed, 74 insertions(+), 78 deletions(-)
>>  * ~1m20s: --local-includes --include-headers-for-types
>>    36 files changed, 70 insertions(+), 72 deletions(-)
>>  * ~1m20s: --local-includes --include-headers-for-types --no-loops
>>    36 files changed, 70 insertions(+), 72 deletions(-)
>
> Where do these changes come from?  I can't reproduce it running your
> script below, and get empty contrib/coccinelle/*.patch files for each
> sets of options.

It's from René's patch in <[email-masked]>
patch in the side-thread.

I'll clarify that commit message in any re-roll, i.e. it should just be
something like: Here's a test of a patch that applies widely to the
tree, showing that using [options] it's the same or different as before,
as expected.

>> And only the "--all-includes --include-headers-for-types" gets the
>> same results as the previous "--all-includes", even with
>> "--local-includes" we miss out on some definitions.
>> 
>> 1. https://lore.kernel.org/git/[email-masked]/
>> 2. https://lore.kernel.org/git/[email-masked]/
>> 3.
>>    for flags in \
>>        '--all-includes' \
>>        '--no-includes' \
>>        '--all-includes --include-headers-for-types' \
>>        '--local-includes --include-headers-for-types' \
>>        '--local-includes --include-headers-for-types --no-loops'
>>    do
>>    	git reset --hard &&
>>            git clean -dxfq contrib/
>>    	time make -j8 coccicheck SPATCH_FLAGS="$flags --patch ." SPATCH_XARGS="xargs -P 8 -n 16" 2>&1 | grep -v SPATCH
>>    	cat contrib/coccinelle/*.patch | git apply
>>    	git --no-pager diff --shortstat
>>    	git --no-pager diff | git patch-id
>>    done