diff mbox series

[v2,2/2] fetch: skip computing output width when not printing anything

Message ID 4b9bbcf7958da6db2be771f910974a6bffb2d94a.1643806143.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series fetch: speed up mirror-fetches with many refs | expand

Commit Message

Patrick Steinhardt Feb. 2, 2022, 12:51 p.m. UTC
When updating references via git-fetch(1), then by default we report to
the user which references have been changed. This output is formatted in
a nice table such that the different columns are aligned. Because the
first column contains abbreviated object IDs we thus need to iterate
over all refs which have changed and compute the minimum length for
their respective abbreviated hashes. While this effort makes sense in
most cases, it is wasteful when the user passes the `--quiet` flag: we
don't print the summary, but still compute the length.

Skip computing the summary width when the user asked for us to be quiet.
This gives us a small speedup of nearly 10% when doing a dry-run
mirror-fetch in a repository with thousands of references being updated:

    Benchmark 1: git fetch --prune --dry-run +refs/*:refs/* (HEAD~)
      Time (mean ± σ):     34.048 s ±  0.233 s    [User: 30.739 s, System: 4.640 s]
      Range (min … max):   33.785 s … 34.296 s    5 runs

    Benchmark 2: git fetch --prune --dry-run +refs/*:refs/* (HEAD)
      Time (mean ± σ):     30.768 s ±  0.287 s    [User: 27.534 s, System: 4.565 s]
      Range (min … max):   30.432 s … 31.181 s    5 runs

    Summary
      'git fetch --prune --dry-run +refs/*:refs/* (HEAD)' ran
        1.11 ± 0.01 times faster than 'git fetch --prune --dry-run +refs/*:refs/* (HEAD~)'

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fetch.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Christian Couder Feb. 9, 2022, 6:10 p.m. UTC | #1
On Wed, Feb 2, 2022 at 5:13 PM Patrick Steinhardt <ps@pks.im> wrote:

> Skip computing the summary width when the user asked for us to be quiet.

There is a --quiet option for git fetch, so here we can expect that it
will be used to test this speedup...

> This gives us a small speedup of nearly 10% when doing a dry-run
> mirror-fetch in a repository with thousands of references being updated:
>
>     Benchmark 1: git fetch --prune --dry-run +refs/*:refs/* (HEAD~)
>       Time (mean ± σ):     34.048 s ±  0.233 s    [User: 30.739 s, System: 4.640 s]
>       Range (min … max):   33.785 s … 34.296 s    5 runs
>
>     Benchmark 2: git fetch --prune --dry-run +refs/*:refs/* (HEAD)
>       Time (mean ± σ):     30.768 s ±  0.287 s    [User: 27.534 s, System: 4.565 s]
>       Range (min … max):   30.432 s … 31.181 s    5 runs
>
>     Summary
>       'git fetch --prune --dry-run +refs/*:refs/* (HEAD)' ran
>         1.11 ± 0.01 times faster than 'git fetch --prune --dry-run +refs/*:refs/* (HEAD~)'

...but --prune and --dry-run are used for testing. It would be nice if
this discrepancy was explained a bit.

Otherwise the commit message and code look good to me.
Patrick Steinhardt Feb. 10, 2022, 12:23 p.m. UTC | #2
On Wed, Feb 09, 2022 at 07:10:38PM +0100, Christian Couder wrote:
> On Wed, Feb 2, 2022 at 5:13 PM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > Skip computing the summary width when the user asked for us to be quiet.
> 
> There is a --quiet option for git fetch, so here we can expect that it
> will be used to test this speedup...
> 
> > This gives us a small speedup of nearly 10% when doing a dry-run
> > mirror-fetch in a repository with thousands of references being updated:
> >
> >     Benchmark 1: git fetch --prune --dry-run +refs/*:refs/* (HEAD~)
> >       Time (mean ± σ):     34.048 s ±  0.233 s    [User: 30.739 s, System: 4.640 s]
> >       Range (min … max):   33.785 s … 34.296 s    5 runs
> >
> >     Benchmark 2: git fetch --prune --dry-run +refs/*:refs/* (HEAD)
> >       Time (mean ± σ):     30.768 s ±  0.287 s    [User: 27.534 s, System: 4.565 s]
> >       Range (min … max):   30.432 s … 31.181 s    5 runs
> >
> >     Summary
> >       'git fetch --prune --dry-run +refs/*:refs/* (HEAD)' ran
> >         1.11 ± 0.01 times faster than 'git fetch --prune --dry-run +refs/*:refs/* (HEAD~)'
> 
> ...but --prune and --dry-run are used for testing. It would be nice if
> this discrepancy was explained a bit.
> 
> Otherwise the commit message and code look good to me.

Yeah, I was hiding away the `--quiet` flag here by accident. I used
`--prune` and `--dry-run` to trigger more lines to be printed to console
and to not take into account the time it takes to update local refs and
fetch objects. But doing so without these flags also demonstrates what I
want to:

    Benchmark 1: git fetch --quiet +refs/*:refs/* (pks-fetch-pack-optim-v1~)
      Time (mean ± σ):     96.078 s ±  0.508 s    [User: 91.378 s, System: 10.870 s]
      Range (min … max):   95.449 s … 96.760 s    5 runs

    Benchmark 2: git fetch --quiet +refs/*:refs/* (pks-fetch-pack-optim-v1)
      Time (mean ± σ):     88.214 s ±  0.192 s    [User: 83.274 s, System: 10.978 s]
      Range (min … max):   87.998 s … 88.446 s    5 runs

So again, I'll update the commit message.

Thanks for your feedback!

Patrick
diff mbox series

Patch

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5f06b21f8e..ebbde5d56d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1093,12 +1093,15 @@  static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct ref *rm;
 	char *url;
 	int want_status;
-	int summary_width = transport_summary_width(ref_map);
+	int summary_width = 0;
 
 	rc = open_fetch_head(&fetch_head);
 	if (rc)
 		return -1;
 
+	if (verbosity >= 0)
+		summary_width = transport_summary_width(ref_map);
+
 	if (raw_url)
 		url = transport_anonymize_url(raw_url);
 	else
@@ -1344,7 +1347,6 @@  static int prune_refs(struct refspec *rs, struct ref *ref_map,
 	int url_len, i, result = 0;
 	struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
 	char *url;
-	int summary_width = transport_summary_width(stale_refs);
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)")
 		: _("   (%s has become dangling)");
@@ -1373,6 +1375,8 @@  static int prune_refs(struct refspec *rs, struct ref *ref_map,
 	}
 
 	if (verbosity >= 0) {
+		int summary_width = transport_summary_width(stale_refs);
+
 		for (ref = stale_refs; ref; ref = ref->next) {
 			struct strbuf sb = STRBUF_INIT;
 			if (!shown_url) {