Re: What's cooking in git.git (Jun 2016, #05; Thu, 16)

[Joey Hess <id@joeyh.name>]

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

Torsten Bögershausen wrote re jh/clean-smudge-annex:
> The thing is that we need to check the file system to find .gitatttibutes,
> even if we just did it 1 nanosecond ago.
> 
> So the .gitattributes is done 3 times:
> -1 would_convert_to_git_filter_fd(
> -2 assert(would_convert_to_git_filter_fd(path));
> -3 convert.c/convert_to_git_filter_fd()
> 
> The only situation where this could be useful is when the .gitattributes
> change between -1 and -2,
> but then they would have changed between -1 and -3, and convert.c
> will die().
> 
> Does it make sense to remove -2 ?

There's less redundant work going on than at first seems, because 
.gitattribute files are only read once and cached. Verified by strace.

So, the redundant work is only in the processing that convert_attrs() does
of the cached .gitattributes.

Notice that there was a similar redundant triple call to convert_attrs()
before my patch set:

1. index_fd checks would_convert_to_git_filter_fd
2. index_stream_convert_blob does assert(would_convert_to_git_filter_fd(path))
   (Again redundantly since 1. is its only caller and has already
   checked.)
3. in convert_to_git_filter_fd

If convert_attrs() is somehow expensive, it might be worth passing a
struct conv_attrs * into the functions that currently call
convert_attrs(). But it does not look expensive to me.

I think it would be safe enough to remove both asserts, at least as the
code is structured now.

-- 
see shy jo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 811 bytes --]