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

[PATCH] ref-filter: format iteratively with lexicographic refname sorting

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 16, 2024, 06:00 UTC
Message-ID
<a873ed828ccae426214cc8f87610df97ff9a269e.1729055871.git.ps@pks.im>

In bd98f9774e (ref-filter.c: filter & format refs in the same callback, 2023-11-14), we have introduced logic into the ref-filter subsystem that determines whether or not we can output references iteratively instead of first collecting all references, post-processing them and printing them once done. This has the advantage that we don't have to store all refs in memory and, when used with e.g. `--count=1`, that we don't have to read all refs in the first place.

One restriction we have in place for that is that caller must not ask for sorted refs, because there is no way to sort the refs without first reading them all into an array. So the benefits can only be reaped when explicitly asking for output not to be sorted.

But there is one exception here where we _can_ get away with sorting refs while streaming: ref backends sort references returned by their iterators in lexicographic order. So if the following conditions are all true we can do iterative streaming:

  - The caller uses at most a single name pattern. Otherwise we'd have
    to sort results from multiple invocations of the iterator.
  - There must be at most a single sorting specification, as otherwise
    we're not using plain lexicographic ordering.
  - The sorting specification must use the "refname".
  - The sorting specification must not be using any flags, like
    case-insensitive sorting.

Now the resulting logic does feel quite fragile overall, which makes me a bit uneasy. But after thinking about this for a while I couldn't find any obvious gaps in my reasoning. Furthermore, given that lexicographic sorting order is the default in git-for-each-ref(1), this is likely to benefit a whole lot of usecases out there.

The following benchmark executes git-for-each-ref(1) in a crafted repo with 1 million references:

  Benchmark 1: git for-each-ref (revision = HEAD~)
    Time (mean ± σ):      6.756 s ±  0.014 s    [User: 3.004 s, System: 3.541 s]
    Range (min … max):    6.738 s …  6.784 s    10 runs
  Benchmark 2: git for-each-ref (revision = HEAD)
    Time (mean ± σ):      6.479 s ±  0.017 s    [User: 2.858 s, System: 3.422 s]
    Range (min … max):    6.450 s …  6.519 s    10 runs
  Summary
    git for-each-ref (revision = HEAD)
      1.04 ± 0.00 times faster than git for-each-ref (revision = HEAD~)

The change results in a slight performance improvement, but nothing that would really stand out. Something that cannot be seen in the benchmark though is peak memory usage, which went from 404.5MB to 68.96kB.

A more interesting benchmark is printing a single referenence with `--count=1`:

  Benchmark 1: git for-each-ref --count=1 (revision = HEAD~)
    Time (mean ± σ):      6.655 s ±  0.018 s    [User: 2.865 s, System: 3.576 s]
    Range (min … max):    6.630 s …  6.680 s    10 runs
  Benchmark 2: git for-each-ref --count=1 (revision = HEAD)
    Time (mean ± σ):       8.6 ms ±   1.3 ms    [User: 2.3 ms, System: 6.1 ms]
    Range (min … max):     6.7 ms …  14.4 ms    266 runs
  Summary
    git git for-each-ref --count=1 (revision = HEAD)
    770.58 ± 116.19 times faster than git for-each-ref --count=1 (revision = HEAD~)

Whereas we scaled with the number of references before, we now print the first reference and exit immediately, which provides a massive win.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---

I'm honestly not quite sure whether I think that this change is fine, or whether it is getting too fragile. I decided to send the patch anyway so that we can discuss on the mailing list, mostly because I think that the results speak for themselves.

Ultimately, this very much feels like a tradeoff to me.
Thanks!
Patrick
 ref-filter.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index dd195007ce1..e075ca21d8e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3244,10 +3244,40 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
 	return ret;
 }
 
+struct ref_sorting {
+	struct ref_sorting *next;
+	int atom; /* index into used_atom array (internal) */
+	enum ref_sorting_order sort_flags;
+};
+
 static inline int can_do_iterative_format(struct ref_filter *filter,
 					  struct ref_sorting *sorting,
 					  struct ref_format *format)
 {
+	/*
+	 * Reference backends sort patterns lexicographically by refname, so if
+	 * the sorting options ask for exactly that we may be able to do
+	 * iterative formatting.
+	 */
+	if (sorting) {
+		size_t n = 0;
+
+		/*
+		 * There must be a single sorting filter that uses
+		 * lexicographic sorting of the refname.
+		 */
+		if (sorting->next ||
+		    sorting->sort_flags ||
+		    used_atom[sorting->atom].atom_type != ATOM_REFNAME)
+			return 0;
+
+		/* And there must be at most a single name pattern. */
+		while (filter->name_patterns && filter->name_patterns[n] && n < 2)
+			n++;
+		if (n > 1)
+			return 0;
+	}
+
 	/*
 	 * Filtering & formatting results within a single ref iteration
 	 * callback is not compatible with options that require
@@ -3258,7 +3288,6 @@ static inline int can_do_iterative_format(struct ref_filter *filter,
 	 */
 	return !(filter->reachable_from ||
 		 filter->unreachable_from ||
-		 sorting ||
 		 format->bases.nr ||
 		 format->is_base_tips.nr);
 }
@@ -3316,12 +3345,6 @@ static int memcasecmp(const void *vs1, const void *vs2, size_t n)
 	return 0;
 }
 
-struct ref_sorting {
-	struct ref_sorting *next;
-	int atom; /* index into used_atom array (internal) */
-	enum ref_sorting_order sort_flags;
-};
-
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
-- 
2.47.0.72.gef8ce8f3d4.dirty
Next: Taylor Blau
Message 1 of 12 in “ref-filter: format iteratively with lexicographic refname sorting”
  1. ref-filter: format iteratively with lexicographic refname sortingPatrick Steinhardt, Oct 16, 2024
  2. Taylor BlauOct 16, 2024
  3. Jeff KingOct 17, 2024
  4. Patrick SteinhardtOct 17, 2024
  5. karthik nayakOct 21, 2024
  6. Taylor BlauOct 21, 2024
  7. ref-filter: format iteratively with lexicographic refname sortingPatrick Steinhardt, Oct 17, 2024
  8. Taylor BlauOct 17, 2024
  9. Toon ClaesOct 21, 2024
  10. Patrick SteinhardtOct 21, 2024
  11. ref-filter: format iteratively with lexicographic refname sortingPatrick Steinhardt, Oct 21, 2024
  12. Taylor BlauOct 21, 2024

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.