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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html