[Qemu-devel] [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Qemu-devel] [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Blue Swirl]

Add logic to detect changes in generated files. If the old
and new files are identical, don't touch the generated file.
This avoids a lot of churn since many files depend on trace.h.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index ff39025..085e8ed 100644
--- a/Makefile
+++ b/Makefile
@@ -107,10 +107,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)

 trace.h: $(SRC_PATH)/trace-events config-host.mak
-	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h
< $< > $@,"  GEN   $@")
+	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h
< $< > $@.tmp,"  GEN   $@")
+	@if test -f $@; then \
+	  if ! cmp -s $@ $@.tmp; then \
+	    mv $@.tmp $@; \
+	  fi; \
+	 else \
+	  mv $@.tmp $@; \
+	 fi

 trace.c: $(SRC_PATH)/trace-events config-host.mak
-	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c
< $< > $@,"  GEN   $@")
+	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c
< $< > $@.tmp,"  GEN   $@")
+	@if test -f $@; then \
+	  if ! cmp -s $@ $@.tmp; then \
+	    mv $@.tmp $@; \
+	  fi; \
+	 else \
+	  mv $@.tmp $@; \
+	 fi

 trace.o: trace.c $(GENERATED_HEADERS)

-- 
1.6.2.4

Re: [Qemu-devel] [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Stefan Hajnoczi]

On Sun, Sep 26, 2010 at 10:05 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
> Add logic to detect changes in generated files. If the old
> and new files are identical, don't touch the generated file.
> This avoids a lot of churn since many files depend on trace.h.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  Makefile |   18 ++++++++++++++++--
>  1 files changed, 16 insertions(+), 2 deletions(-)

Looks good.

Stefan

Re: [Qemu-devel] [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> Add logic to detect changes in generated files. If the old
> and new files are identical, don't touch the generated file.
> This avoids a lot of churn since many files depend on trace.h.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  Makefile |   18 ++++++++++++++++--
>  1 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ff39025..085e8ed 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -107,10 +107,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>  bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
>
>  trace.h: $(SRC_PATH)/trace-events config-host.mak
> -	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h
> < $< > $@,"  GEN   $@")
> +	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h
> < $< > $@.tmp,"  GEN   $@")
> +	@if test -f $@; then \
> +	  if ! cmp -s $@ $@.tmp; then \
> +	    mv $@.tmp $@; \
> +	  fi; \
> +	 else \
> +	  mv $@.tmp $@; \
> +	 fi

Why the conditional?  cmp -s fails fine when argument files don't exist.

Note that common versions of make including GNU Make do not stat a
rule's target to check whether the rule changed it.  Instead, they
assume it changed, and remake everything depending on it.

    armbru@blackfin:~/tmp$ cat Makefile 
    foo: bar
            echo "Remaking foo"

    bar:
            [ -f $@ ] || touch $@ && echo "Touched bar"
    armbru@blackfin:~/tmp$ rm -f foo
    armbru@blackfin:~/tmp$ make
    [ -f bar ] || touch bar && echo "Touched bar"
    Touched bar
    echo "Remaking foo"
    Remaking foo
    armbru@blackfin:~/tmp$ make
    echo "Remaking foo"
    Remaking foo

I doubt your patch avoids churn as advertized with such makes.

[...]

[Qemu-devel] Re: [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Paolo Bonzini]

On 09/27/2010 10:32 AM, Markus Armbruster wrote:
> Why the conditional?  cmp -s fails fine when argument files don't exist.
> 
> Note that common versions of make including GNU Make do not stat a
> rule's target to check whether the rule changed it.  Instead, they
> assume it changed, and remake everything depending on it.
> 
>      armbru@blackfin:~/tmp$ cat Makefile
>      foo: bar
>              echo "Remaking foo"
> 
>      bar:
>              [ -f $@ ] || touch $@&&  echo "Touched bar"
>      armbru@blackfin:~/tmp$ rm -f foo
>      armbru@blackfin:~/tmp$ make
>      [ -f bar ] || touch bar&&  echo "Touched bar"
>      Touched bar
>      echo "Remaking foo"
>      Remaking foo
>      armbru@blackfin:~/tmp$ make
>      echo "Remaking foo"
>      Remaking foo
> 
> I doubt your patch avoids churn as advertized with such makes.

Indeed, see how it's done for config-*.h.

# Uses generic rule in rules.mak
trace.h: trace.h-timestamp
trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak
	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@,"  GEN  trace.h)
	@cmp $@ trace.h >/dev/null 2>&1 || cp $@ trace.h

(untested).

Paolo

[Qemu-devel] Re: [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Blue Swirl]

On Mon, Sep 27, 2010 at 9:51 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 09/27/2010 10:32 AM, Markus Armbruster wrote:
>> Why the conditional?  cmp -s fails fine when argument files don't exist.
>>
>> Note that common versions of make including GNU Make do not stat a
>> rule's target to check whether the rule changed it.  Instead, they
>> assume it changed, and remake everything depending on it.
>>
>>      armbru@blackfin:~/tmp$ cat Makefile
>>      foo: bar
>>              echo "Remaking foo"
>>
>>      bar:
>>              [ -f $@ ] || touch $@&&  echo "Touched bar"
>>      armbru@blackfin:~/tmp$ rm -f foo
>>      armbru@blackfin:~/tmp$ make
>>      [ -f bar ] || touch bar&&  echo "Touched bar"
>>      Touched bar
>>      echo "Remaking foo"
>>      Remaking foo
>>      armbru@blackfin:~/tmp$ make
>>      echo "Remaking foo"
>>      Remaking foo
>>
>> I doubt your patch avoids churn as advertized with such makes.
>
> Indeed, see how it's done for config-*.h.
>
> # Uses generic rule in rules.mak
> trace.h: trace.h-timestamp
> trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak
>        $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@,"  GEN  trace.h)
>        @cmp $@ trace.h >/dev/null 2>&1 || cp $@ trace.h
>
> (untested).

I just copied the rule from %/config-devices.mak. Is also that rule
then incorrect?

Perhaps there could be a macro for this, not unlike move-if-change
script in various GNU packages?

[Qemu-devel] Re: [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Paolo Bonzini]

On 09/27/2010 06:13 PM, Blue Swirl wrote:
>> Indeed, see how it's done for config-*.h.
>>
>> # Uses generic rule in rules.mak
>> trace.h: trace.h-timestamp
>> trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak
>>         $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h<  $<  >  $@,"  GEN  trace.h)
>>         @cmp $@ trace.h>/dev/null 2>&1 || cp $@ trace.h
>>
>> (untested).
>
> I just copied the rule from %/config-devices.mak. Is also that rule
> then incorrect?

config-devices.mak will cause the Makefile to be reread, but no 
recompilations, so it's fine not to use a timestamp file.  Headers are 
more complicated.

> Perhaps there could be a macro for this, not unlike move-if-change
> script in various GNU packages?

The problem is not really the move-if-change script, but rather the 
timestamping rules.  You can use $(eval) for that but it quickly becomes 
unmanageable.

Paolo

Re: [Qemu-devel] [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> Add logic to detect changes in generated files. If the old
>> and new files are identical, don't touch the generated file.
>> This avoids a lot of churn since many files depend on trace.h.
>>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>>  Makefile |   18 ++++++++++++++++--
>>  1 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index ff39025..085e8ed 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -107,10 +107,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>>  bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
>>
>>  trace.h: $(SRC_PATH)/trace-events config-host.mak
>> -	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h
>> < $< > $@,"  GEN   $@")
>> +	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h
>> < $< > $@.tmp,"  GEN   $@")
>> +	@if test -f $@; then \
>> +	  if ! cmp -s $@ $@.tmp; then \
>> +	    mv $@.tmp $@; \
>> +	  fi; \
>> +	 else \
>> +	  mv $@.tmp $@; \
>> +	 fi
>
> Why the conditional?  cmp -s fails fine when argument files don't exist.
>
> Note that common versions of make including GNU Make do not stat a
> rule's target to check whether the rule changed it.  Instead, they
> assume it changed, and remake everything depending on it.

That was from memory.  And my example makefile to illustrate the problem
was flawed.  So your patch might work reliably after all.  The common
make voodoo incantation for this is a stamp file, though, as Paolo
pointed out.

[...]

[Qemu-devel] Re: [PATCH 2/2] trace: avoid unnecessary recompilation if nothing changed

[Paolo Bonzini]

On 09/28/2010 10:58 AM, Markus Armbruster wrote:
> That was from memory.  And my example makefile to illustrate the problem
> was flawed.  So your patch might work reliably after all.  The common
> make voodoo incantation for this is a stamp file, though, as Paolo
> pointed out.

FWIW, Autoconf manual says that the stamp file is needed for 
low-resolution (1s or, for FAT, 2s) filesystems or something like that.

Paolo