Re: [Gnu-arch-users] Re: [PATCH] arch speedups on big trees

[Aaron Bentley]

Tom Lord wrote:

>    > From: Aaron Bentley <address@hidden>
>
>    > I think there's a bit of crosstalk-- as I read it, Miles wants to avoid
>    > creating the tmp even when diff has output, because renames are
>    > expensive on NFS.
>
> Not just renames -- actually creating and removing the file, even if
> it is not renamed, is more expense than needed.   If all FS operations
> are expensive -- the tmp file should never exist at all unless we know
> it is needed.
>  
Agreed, but that is taken care of.  See below.

> There's a function in `src/tla/libarch/make-change.c' called
> `emit_file_or_symlink_patches' that creates the file for diff output
> before figuring out if there will _be_ any diff output.  It deletes
> the file if diff invocation exits with status 0.  At the same time,
> the only way that diff is ever invoked, from `arch_invoke_diff',
> doesn't bother to actually invoke diff unless it is certain that there
> _will_ be output (this was a big performance improvement a while
> back).  Simply deferring the creation of the file until it is certain
> that diff will actually run will do the trick.  It's a simple
> refactoring.
>  
Chris' patch does that.  He's changed arch_invoke_diff's first argument 
to a pointer, added a filename argument, and created the file only if 
arch_binary_files_differ() returns true.

So that leaves the case where the file *should* be created, but is being 
created with the wrong name.  But we have all the required data to 
create it with the right name in the first place, and delete if 
diff_stat!=1.

Unfortunately, this adds a requirement to ensure the directory exists 
during arch_invoke_diff, and it also means that the changeset can 
temporarily contain bad .diff files. 

Really, I think the call to binary_files_differ() should be moved into 
emit_file_or_symlink_patches(), so we can return early if there's 
nothing to do.

Aaron