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

[PATCH 3/6] Ref-count the filespecs used by diffcore

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Oct 25, 2007, 18:19 UTC
Message-ID
<alpine.LFD.0.999.0710251117590.30120@woody.linux-foundation.org>
In-Reply-To
<alpine.LFD.0.999.0710251112120.30120@woody.linux-foundation.org>
From: Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 3/6] Ref-count the filespecs used by diffcore

Rather than copy the refcounts when introducing new versions of them (for rename or copy detection), use a refcount and increment the count when reusing the diff_filespec.

This avoids unnecessary allocations, but the real reason behind this is a future enhancement: we will want to track shared data across the copy/rename detection. In order to efficiently notice when a filespec is used by a rename, the rename machinery wants to keep track of a rename usage count which is shared across all different users of the filespec.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This was sent out in the original series as a fix on top of broken code. Now it sets up the infrastructure so that the next patch won't be broken.

 diff.c            |   15 +++++++++++----
 diffcore-rename.c |   16 ++++++----------
 diffcore.h        |    2 ++
 3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/diff.c b/diff.c
index dfb8595..0b320f6 100644
--- a/diff.c
+++ b/diff.c
@@ -1440,9 +1440,18 @@ struct diff_filespec *alloc_filespec(const char *path)
 	memset(spec, 0, sizeof(*spec));
 	spec->path = (char *)(spec + 1);
 	memcpy(spec->path, path, namelen+1);
+	spec->count = 1;
 	return spec;
 }
 
+void free_filespec(struct diff_filespec *spec)
+{
+	if (!--spec->count) {
+		diff_free_filespec_data(spec);
+		free(spec);
+	}
+}
+
 void fill_filespec(struct diff_filespec *spec, const unsigned char *sha1,
 		   unsigned short mode)
 {
@@ -2435,10 +2444,8 @@ struct diff_filepair *diff_queue(struct diff_queue_struct *queue,
 
 void diff_free_filepair(struct diff_filepair *p)
 {
-	diff_free_filespec_data(p->one);
-	diff_free_filespec_data(p->two);
-	free(p->one);
-	free(p->two);
+	free_filespec(p->one);
+	free_filespec(p->two);
 	free(p);
 }
 
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 2077a9b..3da06b7 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -209,21 +209,19 @@ static int estimate_similarity(struct diff_filespec *src,
 
 static void record_rename_pair(int dst_index, int src_index, int score)
 {
-	struct diff_filespec *one, *two, *src, *dst;
+	struct diff_filespec *src, *dst;
 	struct diff_filepair *dp;
 
 	if (rename_dst[dst_index].pair)
 		die("internal error: dst already matched.");
 
 	src = rename_src[src_index].one;
-	one = alloc_filespec(src->path);
-	fill_filespec(one, src->sha1, src->mode);
+	src->count++;
 
 	dst = rename_dst[dst_index].two;
-	two = alloc_filespec(dst->path);
-	fill_filespec(two, dst->sha1, dst->mode);
+	dst->count++;
 
-	dp = diff_queue(NULL, one, two);
+	dp = diff_queue(NULL, src, dst);
 	dp->renamed_pair = 1;
 	if (!strcmp(src->path, dst->path))
 		dp->score = rename_src[src_index].score;
@@ -526,10 +524,8 @@ void diffcore_rename(struct diff_options *options)
 		}
 	}
 
-	for (i = 0; i < rename_dst_nr; i++) {
-		diff_free_filespec_data(rename_dst[i].two);
-		free(rename_dst[i].two);
-	}
+	for (i = 0; i < rename_dst_nr; i++)
+		free_filespec(rename_dst[i].two);
 
 	free(rename_dst);
 	rename_dst = NULL;
diff --git a/diffcore.h b/diffcore.h
index eb618b1..30055ac 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -29,6 +29,7 @@ struct diff_filespec {
 	void *cnt_data;
 	const char *funcname_pattern_ident;
 	unsigned long size;
+	int count;               /* Reference count */
 	int xfrm_flags;		 /* for use by the xfrm */
 	unsigned short mode;	 /* file mode */
 	unsigned sha1_valid : 1; /* if true, use sha1 and trust mode;
@@ -43,6 +44,7 @@ struct diff_filespec {
 };
 
 extern struct diff_filespec *alloc_filespec(const char *);
+extern void free_filespec(struct diff_filespec *);
 extern void fill_filespec(struct diff_filespec *, const unsigned char *,
 			  unsigned short);
 
-- 
1.5.3.4.330.g1dec6
Previous: Linus TorvaldsNext: Linus Torvalds
Message 4 of 18 in “Cleaned-up rename detection patch-series”
  1. 0/6 Cleaned-up rename detection patch-seriesLinus Torvalds, Oct 25, 2007
  2. 1/6 Add 'diffcore.h' to LIB_HLinus Torvalds, Oct 25, 2007
  3. 2/6 Split out "exact content match" phase of rename detectionLinus Torvalds, Oct 25, 2007
  4. 3/6 Ref-count the filespecs used by diffcoreLinus Torvalds, Oct 25, 2007
  5. 4/6 copy vs rename detection: avoid unnecessary O(n*m) loopsLinus Torvalds, Oct 25, 2007
  6. Junio C HamanoOct 26, 2007
  7. Linus TorvaldsOct 26, 2007
  8. Junio C HamanoOct 26, 2007
  9. Linus TorvaldsOct 26, 2007
  10. 5/6 Do linear-time/space rename logic for exact renamesLinus Torvalds, Oct 25, 2007
  11. Daniel BarkalowOct 25, 2007
  12. Jeff KingOct 25, 2007
  13. Daniel BarkalowOct 25, 2007
  14. Linus TorvaldsOct 25, 2007
  15. Daniel BarkalowOct 25, 2007
  16. 6/6 Do exact rename detection regardless of rename limitsLinus Torvalds, Oct 25, 2007
  17. Linus TorvaldsOct 25, 2007
  18. Jeff KingOct 25, 2007

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.