Web lists-archives.com

Re: [PATCH v3] coccicheck: process every source file at once




On 07.10.18 13:36, Beat Bolli wrote:
> On 02.10.18 22:18, Jacob Keller wrote:
>> On Tue, Oct 2, 2018 at 1:07 PM Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
>>>
>>> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>>>
>>> make coccicheck is used in order to apply coccinelle semantic patches,
>>> and see if any of the transformations found within contrib/coccinelle/
>>> can be applied to the current code base.
>>>
>>> Pass every file to a single invocation of spatch, instead of running
>>> spatch once per source file.
>>>
>>> This reduces the time required to run make coccicheck by a significant
>>> amount of time:
>>>
>>> Prior timing of make coccicheck
>>>   real    6m14.090s
>>>   user    25m2.606s
>>>   sys     1m22.919s
>>>
>>> New timing of make coccicheck
>>>   real    1m36.580s
>>>   user    7m55.933s
>>>   sys     0m18.219s
>>>
>>> This is nearly a 4x decrease in the time required to run make
>>> coccicheck. This is due to the overhead of restarting spatch for every
>>> file. By processing all files at once, we can amortize this startup cost
>>> across the total number of files, rather than paying it once per file.
>>>
>>> Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
>>> ---
>>
>> Forgot to add what changed. I dropped the subshell and "||" bit around
>> invoking spatch.
>>
>> Thanks,
>> Jake
>>
>>
>>>  Makefile | 6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index df1df9db78da..da692ece9e12 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -2715,10 +2715,8 @@ endif
>>>  %.cocci.patch: %.cocci $(COCCI_SOURCES)
>>>         @echo '    ' SPATCH $<; \
>>>         ret=0; \
>>> -       for f in $(COCCI_SOURCES); do \
>>> -               $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
>>> -                       { ret=$$?; break; }; \
>>> -       done >$@+ 2>$@.log; \
>>> +       $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) >$@+ 2>$@.log; \
>>> +       ret=$$?; \
>>>         if test $$ret != 0; \
>>>         then \
>>>                 cat $@.log; \
>>> --
>>> 2.18.0.219.gaf81d287a9da
>>>
> 
> Wouldn't the following be even simpler?
> 
> diff --git a/Makefile b/Makefile
> index 5c8307b7c479..a37b2724d526 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2701,12 +2701,7 @@ endif
> 
>  %.cocci.patch: %.cocci $(COCCI_SOURCES)
>         @echo '    ' SPATCH $<; \
> -       ret=0; \
> -       for f in $(COCCI_SOURCES); do \
> -               $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
> -                       { ret=$$?; break; }; \
> -       done >$@+ 2>$@.log; \
> -       if test $$ret != 0; \
> +       if ! $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) 
>> $@+ 2>$@.log; \

The "If !" and the output redirection should be on one line,
obviously... Sorry about this.

Beat