Re: [Qemu-devel] [PATCH RFC 08/21] qapi: Touch generated files only when they change

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]

On 02/02/2018 07:03 AM, Markus Armbruster wrote:
> A massive number of objects depends on QAPI-generated headers.  In my
> "build everything" tree, it's roughly 4500 out of 4800.  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 4500 objects.  But adding a QAPI type is as bad as ever; we
> clearly got more work to do.

The last phrase sounds quite colloquial.  It may have been intentional;
but if not, s/we/we've/

> 
> Signed-off-by: Markus Armbruster <armbru@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 cfa2671ca3..be0fcc548a 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1944,9 +1944,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)

In-memory rewrite rather than playing games with a secondary file
combined with the rename(2) syscall.  It means you're not atomic (an
external reader has a window of time where they can see an incomplete
version of the file).  But I think make dependency rules mean we don't
care - even in a parallel rule, as long as all clients depend on the
timestamp file, and the timestamp file isn't updated until all our
rewrites (if any) have settled, then we don't create any of those
external readers that can find the window of incomplete contents.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]