Re: Performance problems in diffcore rename
From: Shawn Pearce <hidden>
Date: 2016-08-11 19:25:37
Possibly related (same subject, not in this thread)
- 2016-08-11 · Performance problems in diffcore rename · Shawn Pearce <hidden>
Junio C Hamano [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Shawn Pearce [off-list ref] writes:quoted
But looking at is_exact_match I'm now wondering why we bother to open the working tree file at all here.You are right. When we _know_ both SHA-1 can be trusted, we should be able to do just this, regardless of contents_too (whose only purpose is to delay comparing a cache dirty working tree file with something else):diff --git a/diffcore-rename.c b/diffcore-rename.c index 57a74b6..677db85 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c@@ -100,9 +100,8 @@ static int is_exact_match(struct diff_filespec *src, struct diff_filespec *dst, int contents_too) { - if (src->sha1_valid && dst->sha1_valid && - !hashcmp(src->sha1, dst->sha1)) - return 1; + if (src->sha1_valid && dst->sha1_valid) + return !hashcmp(src->sha1, dst->sha1); if (!contents_too) return 0; if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
OK, I've stared at this problem for hours. The above patch breaks a number of test cases (such as ./t4005-diff-rename-2.sh) as it causes the rename/copy detection to come out differently. It turns out the difference is not filling in the size field of struct diff_filespec. By bypassing that step in is_exact_match during the second pass we are altering the outcome. --