diff: do not reuse worktree files that need "clean" conversion

When accessing a blob for a diff, we may try to reuse file contents in the working tree, under the theory that it is faster to mmap those file contents than it would be to extract the content from the object database. When we have to filter those contents, though, that assumption does not hold. Even for our internal conversions like CRLF, we have to allocate and fill a new buffer anyway. But much worse, for external clean filters we have to exec an arbitrary script, and we have no idea how expensive it may be to run. So let's skip this optimization when conversion into git's "clean" form is required. This applies whenever the "want_file" flag is false. When it's true, the caller actually wants the smudged worktree contents, which the reused file by definition already has (in fact, this is a key optimization going the other direction, since reusing the worktree file there lets us skip smudge filters). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jul 22, 2016 at 11:27 UTC 06dec439a3d00fa0b3141a2d52b33d22b941976c
2 files changed +18
diff.c
+7
@@ -2672,6 +2672,13 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
2672 if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1))
2673 return 0;
2674
2675 + /*
2676 + * Similarly, if we'd have to convert the file contents anyway, that
2677 + * makes the optimization not worthwhile.
2678 + */
2679 + if (!want_file && would_convert_to_git(name))
2680 + return 0;
2681 +
2682 len = strlen(name);
2683 pos = cache_name_pos(name, len);
2684 if (pos < 0)
t/t0021-conversion.sh
+11
@@ -268,4 +268,15 @@ test_expect_success 'disable filter with empty override' '
268 test_must_be_empty err
269 '
270
271 +test_expect_success 'diff does not reuse worktree files that need cleaning' '
272 + test_config filter.counter.clean "echo . >>count; sed s/^/clean:/" &&
273 + echo "file filter=counter" >.gitattributes &&
274 + test_commit one file &&
275 + test_commit two file &&
276 +
277 + >count &&
278 + git diff-tree -p HEAD &&
279 + test_line_count = 0 count
280 +'
281 +
282 test_done