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

[PATCH 05/15] refs: stop using mkpath() in refname_match()

From
Jon Simons <jon@jonsimons.org>
Date
Oct 9, 2026, 19:29 UTC
Message-ID
<20261009192953.81794-6-jon@jonsimons.org>
In-Reply-To
<20261009192953.81794-1-jon@jonsimons.org>

refname_match() formats every ref_rev_parse_rules entry through mkpath() to then strcmp() with its given full_name. It turns out this formatting function dominates the client-side cost of matching explicit refspecs against a remote that advertises lots of refs.

Reuse match_parse_rule() instead to efficiently compare each rule against its given full_name. With this change we avoid mkpath()'s strbuf_vaddf(), and each match is now a direct prefix + suffix comparison, length check, and memcmp of the refname at hand.

Timings from the test added in the previous commit:
  Test                           HEAD~1            HEAD
  -----------------------------------------------------------------------
  5516.3: empty:refspecs:1       0.15(0.09+0.11)   0.14(0.07+0.11) -6.7%
  5516.5: empty:refspecs:10      0.38(0.32+0.11)   0.17(0.11+0.11) -55.3%
  5516.7: empty:refspecs:100     2.50(2.43+0.11)   0.49(0.42+0.11) -80.4%
  5516.9: mirror:refspecs:1      0.21(0.15+0.11)   0.16(0.09+0.11) -23.8%
  5516.11: mirror:refspecs:10    0.80(0.73+0.11)   0.26(0.19+0.11) -67.5%
  5516.13: mirror:refspecs:100   6.68(6.59+0.13)   1.18(1.11+0.11) -82.3%
Notes on a change in behavior:
 - mkpath() runs cleanup_path(), which strips leading "./".  So,
   "./refs/heads/foo" previously matched "refs/heads/foo" through the
   "%.*s" rule.  As of this commit, this is no longer true.
 - It's considered a bug that "./"-prefixed strings previously could
   have been matched with refnames in this way:
   - cleanup_path() dates back to 26c8a533af (Add "mkpath()" helper
     function, 2005-07-08), and is intended to be used for filepaths,
     not refnames.  The leading-"./" strip itself comes from f17a1b1bec
     (Fix up path-cleanup in git_path() properly, 2005-07-05).
   - refname_match() has used mkpath() in its matching loop since its
     inception with 79803322c1 (add refname_match(), 2007-11-11).
   - 6cd4a8982d (avoid using mksnpath for refs, 2017-03-28) previously
     removed other cleanup_path() spots reachable from mksnpath() and
     notes they were "questionable when dealing with refnames, as we
     could silently canonicalize a syntactically bogus refname into a
     valid one."

Tests in t5510 and t5516 that cover the aliasing behavior are toggled to test_expect_success.

Signed-off-by: Jon Simons <jon@jonsimons.org>
---
 refs.c                | 88 +++++++++++++++++++++++--------------------
 t/t5510-fetch.sh      |  4 +-
 t/t5516-fetch-push.sh |  2 +-
 3 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/refs.c b/refs.c
index 951db56113..9bf3bc8153 100644
--- a/refs.c
+++ b/refs.c
@@ -652,6 +652,44 @@ static const char *ref_rev_parse_rules[] = {
 
 #define NUM_REV_PARSE_RULES (ARRAY_SIZE(ref_rev_parse_rules) - 1)
 
+/*
+ * Check that the string refname matches a rule of the form
+ * "{prefix}%.*s{suffix}". So "foo/bar/baz" would match the rule
+ * "foo/%.*s/baz", and return the string "bar".
+ */
+static const char *match_parse_rule(const char *refname, const char *rule,
+				    size_t *len)
+{
+	/*
+	 * Check that rule matches refname up to the first percent in the rule.
+	 * We can bail immediately if not, but otherwise we leave "rule" at the
+	 * %-placeholder, and "refname" at the start of the potential matched
+	 * name.
+	 */
+	while (*rule != '%') {
+		if (!*rule)
+			BUG("rev-parse rule did not have percent");
+		if (*refname++ != *rule++)
+			return NULL;
+	}
+
+	/*
+	 * Check that our "%" is the expected placeholder. This assumes there
+	 * are no other percents (placeholder or quoted) in the string, but
+	 * that is sufficient for our rev-parse rules.
+	 */
+	if (!skip_prefix(rule, "%.*s", &rule))
+		return NULL;
+
+	/*
+	 * And now check that our suffix (if any) matches.
+	 */
+	if (!strip_suffix(refname, rule, len))
+		return NULL;
+
+	return refname; /* len set by strip_suffix() */
+}
+
 /*
  * Is it possible that the caller meant full_name with abbrev_name?
  * If so return a non-zero value to signal "yes"; the magnitude of
@@ -662,12 +700,18 @@ static const char *ref_rev_parse_rules[] = {
 int refname_match(const char *abbrev_name, const char *full_name)
 {
 	const char **p;
-	const int abbrev_name_len = strlen(abbrev_name);
+	const size_t abbrev_name_len = strlen(abbrev_name);
 	const int num_rules = NUM_REV_PARSE_RULES;
 
-	for (p = ref_rev_parse_rules; *p; p++)
-		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name)))
+	for (p = ref_rev_parse_rules; *p; p++) {
+		size_t short_name_len;
+		const char *short_name = match_parse_rule(full_name, *p,
+							  &short_name_len);
+
+		if (short_name && short_name_len == abbrev_name_len &&
+		    !memcmp(short_name, abbrev_name, abbrev_name_len))
 			return &ref_rev_parse_rules[num_rules] - p;
+	}
 
 	return 0;
 }
@@ -1615,44 +1659,6 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
 	return 0;
 }
 
-/*
- * Check that the string refname matches a rule of the form
- * "{prefix}%.*s{suffix}". So "foo/bar/baz" would match the rule
- * "foo/%.*s/baz", and return the string "bar".
- */
-static const char *match_parse_rule(const char *refname, const char *rule,
-				    size_t *len)
-{
-	/*
-	 * Check that rule matches refname up to the first percent in the rule.
-	 * We can bail immediately if not, but otherwise we leave "rule" at the
-	 * %-placeholder, and "refname" at the start of the potential matched
-	 * name.
-	 */
-	while (*rule != '%') {
-		if (!*rule)
-			BUG("rev-parse rule did not have percent");
-		if (*refname++ != *rule++)
-			return NULL;
-	}
-
-	/*
-	 * Check that our "%" is the expected placeholder. This assumes there
-	 * are no other percents (placeholder or quoted) in the string, but
-	 * that is sufficient for our rev-parse rules.
-	 */
-	if (!skip_prefix(rule, "%.*s", &rule))
-		return NULL;
-
-	/*
-	 * And now check that our suffix (if any) matches.
-	 */
-	if (!strip_suffix(refname, rule, len))
-		return NULL;
-
-	return refname; /* len set by strip_suffix() */
-}
-
 char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 				   const char *refname, int strict)
 {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 0303784b1f..941213f36a 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1074,7 +1074,7 @@ test_expect_success 'LHS of refspec follows ref disambiguation rules' '
 	)
 '
 
-test_expect_failure 'fetch with "./"-prefixed branch.<name>.merge does not mark any ref for merge' '
+test_expect_success 'fetch with "./"-prefixed branch.<name>.merge does not mark any ref for merge' '
 	mkdir dotslash-merge-default-refspec &&
 	(
 		cd dotslash-merge-default-refspec &&
@@ -1096,7 +1096,7 @@ test_expect_failure 'fetch with "./"-prefixed branch.<name>.merge does not mark
 	)
 '
 
-test_expect_failure 'fetch protocol v0 with "./"-prefixed branch.<name>.merge does not match any remote ref' '
+test_expect_success 'fetch protocol v0 with "./"-prefixed branch.<name>.merge does not match any remote ref' '
 	mkdir dotslash-merge-fetch-protocol-v0 &&
 	(
 		cd dotslash-merge-fetch-protocol-v0 &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index aaeb251e2f..81ad6cd52f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -433,7 +433,7 @@ test_expect_success 'push with onelevel ref' '
 	test_must_fail git push testrepo HEAD:refs/onelevel
 '
 
-test_expect_failure 'push with "./"-prefixed src does not match any ref' '
+test_expect_success 'push with "./"-prefixed src does not match any ref' '
 	mk_test testrepo heads/main &&
 	test_must_fail git push testrepo ./refs/heads/main:refs/heads/frotz 2>err &&
 	test_grep "src refspec ./refs/heads/main does not match any" err

-- 
2.55.0
Previous: Jon SimonsNext: Jon Simons
Message 6 of 18 in “push: speed up client-side refspec matching”
  1. 00/15 push: speed up client-side refspec matchingJon Simons, Oct 9, 2026
  2. 01/15 remote: validate --force-with-lease <refname> argumentJon Simons, Oct 9, 2026
  3. 02/15 t5516: demonstrate push with "./"-prefixed sourceJon Simons, Oct 9, 2026
  4. 03/15 t5510: document fetch with "./"-prefixed branch.<name>.mergeJon Simons, Oct 9, 2026
  5. 04/15 t/perf: add explicit delete refspec matching testJon Simons, Oct 9, 2026
  6. 05/15 refs: stop using mkpath() in refname_match()Jon Simons, Oct 9, 2026
  7. 06/15 remote: use strmap for check_push_refs()Jon Simons, Oct 9, 2026
  8. 07/15 t5516: test pushing two refspecs creating the same new branchJon Simons, Oct 9, 2026
  9. 08/15 t5408, t5410: test duplicate updates without relying on the clientJon Simons, Oct 9, 2026
  10. 09/15 t5408: check refspec order with distinct destinationsJon Simons, Oct 9, 2026
  11. 10/15 t5408: expect client-side error for duplicate destinationsJon Simons, Oct 9, 2026
  12. 11/15 remote: reject duplicate destinations on an empty remoteJon Simons, Oct 9, 2026
  13. 12/15 remote: use strmap for match_explicit_refs()Jon Simons, Oct 9, 2026
  14. 13/15 t/perf: measure --force-with-lease in p5516Jon Simons, Oct 9, 2026
  15. 14/15 remote: restructure apply_push_cas() loopsJon Simons, Oct 9, 2026
  16. 15/15 remote: use strmap for apply_push_cas()Jon Simons, Oct 9, 2026
  17. Kristoffer HaugsbakkOct 9, 2026
  18. Jon SimonsOct 11, 2026

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.