diff mbox

[v2,10/29] qapi: Touch generated files only when they change

Message ID 20180211093607.27351-11-armbru@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Markus Armbruster Feb. 11, 2018, 9:35 a.m. UTC
A massive number of objects depends on QAPI-generated headers.  In my
"build everything" tree, it's roughly 4800 out of 5100.  This is
particularly annoying when only some of the generated files change,
say for a doc fix.

Improve qapi-gen.py to touch its output files only if they actually
change.  Rebuild time for a QAPI doc fix drops from many minutes to a
few seconds.  Rebuilds get faster for certain code changes, too.  For
instance, adding a simple QMP event now recompiles less than 200
instead of 4800 objects.  But adding a QAPI type is as bad as ever;
we've clearly got more work to do.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi/common.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Eric Blake Feb. 12, 2018, 7:48 p.m. UTC | #1
On 02/11/2018 03:35 AM, Markus Armbruster wrote:
> A massive number of objects depends on QAPI-generated headers.  In my
> "build everything" tree, it's roughly 4800 out of 5100.  This is
> particularly annoying when only some of the generated files change,
> say for a doc fix.
> 
> Improve qapi-gen.py to touch its output files only if they actually
> change.  Rebuild time for a QAPI doc fix drops from many minutes to a
> few seconds.  Rebuilds get faster for certain code changes, too.  For
> instance, adding a simple QMP event now recompiles less than 200
> instead of 4800 objects.  But adding a QAPI type is as bad as ever;
> we've clearly got more work to do.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>   scripts/qapi/common.py | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 8290795dc1..2e58573a39 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1951,9 +1951,16 @@ class QAPIGen(object):
>               except os.error as e:
>                   if e.errno != errno.EEXIST:
>                       raise
> -        f = open(os.path.join(output_dir, fname), 'w')
> -        f.write(self._top(fname) + self._preamble + self._body
> +        fd = os.open(os.path.join(output_dir, fname),
> +                     os.O_RDWR | os.O_CREAT, 0666)

patchew complained here for mingw; I'm not sure why.
Marc-André Lureau Feb. 13, 2018, 3:22 p.m. UTC | #2
Hi

On Mon, Feb 12, 2018 at 8:48 PM, Eric Blake <eblake@redhat.com> wrote:
> On 02/11/2018 03:35 AM, Markus Armbruster wrote:
>>
>> A massive number of objects depends on QAPI-generated headers.  In my
>> "build everything" tree, it's roughly 4800 out of 5100.  This is
>> particularly annoying when only some of the generated files change,
>> say for a doc fix.
>>
>> Improve qapi-gen.py to touch its output files only if they actually
>> change.  Rebuild time for a QAPI doc fix drops from many minutes to a
>> few seconds.  Rebuilds get faster for certain code changes, too.  For
>> instance, adding a simple QMP event now recompiles less than 200
>> instead of 4800 objects.  But adding a QAPI type is as bad as ever;
>> we've clearly got more work to do.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>   scripts/qapi/common.py | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
>> index 8290795dc1..2e58573a39 100644
>> --- a/scripts/qapi/common.py
>> +++ b/scripts/qapi/common.py
>> @@ -1951,9 +1951,16 @@ class QAPIGen(object):
>>               except os.error as e:
>>                   if e.errno != errno.EEXIST:
>>                       raise
>> -        f = open(os.path.join(output_dir, fname), 'w')
>> -        f.write(self._top(fname) + self._preamble + self._body
>> +        fd = os.open(os.path.join(output_dir, fname),
>> +                     os.O_RDWR | os.O_CREAT, 0666)
>
>
> patchew complained here for mingw; I'm not sure why.

python3 syntax error.
https://stackoverflow.com/questions/1837874/invalid-token-when-using-octal-numbers

>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
Michael Roth Feb. 18, 2018, 10:43 p.m. UTC | #3
Quoting Markus Armbruster (2018-02-11 03:35:48)
> A massive number of objects depends on QAPI-generated headers.  In my
> "build everything" tree, it's roughly 4800 out of 5100.  This is
> particularly annoying when only some of the generated files change,
> say for a doc fix.
> 
> Improve qapi-gen.py to touch its output files only if they actually
> change.  Rebuild time for a QAPI doc fix drops from many minutes to a
> few seconds.  Rebuilds get faster for certain code changes, too.  For
> instance, adding a simple QMP event now recompiles less than 200
> instead of 4800 objects.  But adding a QAPI type is as bad as ever;
> we've clearly got more work to do.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  scripts/qapi/common.py | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 8290795dc1..2e58573a39 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1951,9 +1951,16 @@ class QAPIGen(object):
>              except os.error as e:
>                  if e.errno != errno.EEXIST:
>                      raise
> -        f = open(os.path.join(output_dir, fname), 'w')
> -        f.write(self._top(fname) + self._preamble + self._body
> +        fd = os.open(os.path.join(output_dir, fname),
> +                     os.O_RDWR | os.O_CREAT, 0666)
> +        f = os.fdopen(fd, 'r+')
> +        text = (self._top(fname) + self._preamble + self._body
>                  + self._bottom(fname))
> +        oldtext = f.read(len(text) + 1)
> +        if text != oldtext:
> +            f.seek(0)
> +            f.truncate(0)
> +            f.write(text)
>          f.close()
> 
> 
> -- 
> 2.13.6
>
Markus Armbruster Feb. 23, 2018, 5:22 p.m. UTC | #4
Marc-Andre Lureau <mlureau@redhat.com> writes:

> Hi
>
> On Mon, Feb 12, 2018 at 8:48 PM, Eric Blake <eblake@redhat.com> wrote:
>> On 02/11/2018 03:35 AM, Markus Armbruster wrote:
>>>
>>> A massive number of objects depends on QAPI-generated headers.  In my
>>> "build everything" tree, it's roughly 4800 out of 5100.  This is
>>> particularly annoying when only some of the generated files change,
>>> say for a doc fix.
>>>
>>> Improve qapi-gen.py to touch its output files only if they actually
>>> change.  Rebuild time for a QAPI doc fix drops from many minutes to a
>>> few seconds.  Rebuilds get faster for certain code changes, too.  For
>>> instance, adding a simple QMP event now recompiles less than 200
>>> instead of 4800 objects.  But adding a QAPI type is as bad as ever;
>>> we've clearly got more work to do.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>   scripts/qapi/common.py | 11 +++++++++--
>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
>>> index 8290795dc1..2e58573a39 100644
>>> --- a/scripts/qapi/common.py
>>> +++ b/scripts/qapi/common.py
>>> @@ -1951,9 +1951,16 @@ class QAPIGen(object):
>>>               except os.error as e:
>>>                   if e.errno != errno.EEXIST:
>>>                       raise
>>> -        f = open(os.path.join(output_dir, fname), 'w')
>>> -        f.write(self._top(fname) + self._preamble + self._body
>>> +        fd = os.open(os.path.join(output_dir, fname),
>>> +                     os.O_RDWR | os.O_CREAT, 0666)
>>
>>
>> patchew complained here for mingw; I'm not sure why.
>
> python3 syntax error.
> https://stackoverflow.com/questions/1837874/invalid-token-when-using-octal-numbers

Yes, we need to spell the mode 0o666.
diff mbox

Patch

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 8290795dc1..2e58573a39 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1951,9 +1951,16 @@  class QAPIGen(object):
             except os.error as e:
                 if e.errno != errno.EEXIST:
                     raise
-        f = open(os.path.join(output_dir, fname), 'w')
-        f.write(self._top(fname) + self._preamble + self._body
+        fd = os.open(os.path.join(output_dir, fname),
+                     os.O_RDWR | os.O_CREAT, 0666)
+        f = os.fdopen(fd, 'r+')
+        text = (self._top(fname) + self._preamble + self._body
                 + self._bottom(fname))
+        oldtext = f.read(len(text) + 1)
+        if text != oldtext:
+            f.seek(0)
+            f.truncate(0)
+            f.write(text)
         f.close()