git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] Rename detection: Avoid repeated filespec population

From
Björn Steinbrink <b.steinbrink@gmx.de>
Date
Jan 20, 2009, 15:59 UTC
Message-ID
<20090120155957.GA23237@atjola.homenet>

In diffcore_rename, we assume that the blob contents in the filespec aren't required anymore after estimate_similarity has been called and thus we free it. But estimate_similarity might return early when the file sizes differ too much. In that case, cnt_data is never set and the next call to estimate_similarity will populate the filespec again, eventually rereading the same blob over and over again.

To fix that, we first get the blob sizes and only when the blob contents are actually required, and when cnt_data will be set, the full filespec is populated, once.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
This actually affects the copy detection way more than the pure
rename detection, due to the larger number of candidates, but it's the
same code, and I only realized that when I reran the stuff to get some
numbers to show off. ;-)

Test with linux-2.6.git and hot caches: git diff-tree -p db30c705758^2..db30c705758

	| unpatched | patched
------------------------------
plain	|        3s |     3s
-M	|        4s |     3s
-C	|    2m 25s |     9s
-C -C	|    4m 30s |    24s
 diffcore-rename.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 168a95b..0b0d6b8 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -153,9 +153,9 @@ static int estimate_similarity(struct diff_filespec *src,
 	 * is a possible size - we really should have a flag to
 	 * say whether the size is valid or not!)
 	 */
-	if (!src->cnt_data && diff_populate_filespec(src, 0))
+	if (!src->cnt_data && diff_populate_filespec(src, 1))
 		return 0;
-	if (!dst->cnt_data && diff_populate_filespec(dst, 0))
+	if (!dst->cnt_data && diff_populate_filespec(dst, 1))
 		return 0;
 
 	max_size = ((src->size > dst->size) ? src->size : dst->size);
@@ -173,6 +173,11 @@ static int estimate_similarity(struct diff_filespec *src,
 	if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
 		return 0;
 
+	if (!src->cnt_data && diff_populate_filespec(src, 0))
+		return 0;
+	if (!dst->cnt_data && diff_populate_filespec(dst, 0))
+		return 0;
+
 	delta_limit = (unsigned long)
 		(base_size * (MAX_SCORE-minimum_score) / MAX_SCORE);
 	if (diffcore_count_changes(src, dst,
-- 
1.6.1.135.g3cf3b
Next: Jeff King
Message 1 of 6 in “Rename detection: Avoid repeated filespec population”
  1. Rename detection: Avoid repeated filespec populationBjörn Steinbrink, Jan 20, 2009
  2. Jeff KingJan 20, 2009
  3. Jeff KingJan 20, 2009
  4. Björn SteinbrinkJan 21, 2009
  5. Jeff KingJan 21, 2009
  6. Junio C HamanoJan 21, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.