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

["Torsten Bögershausen" <tboegi@web.de>]

On 07/13/2016 12:20 AM, Joey Hess wrote:
> 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.
>
OK, I think I missed that work (not enough time for Git at the moment)
Junio, please help me out, do we have a cache here now?
I tried to figure out that following your attr branch, but failed.
> 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 have that on the list, but seems to be uneccesary now.
>
> I think it would be safe enough to remove both asserts, at least as the
> code is structured now.
>
OK.