[PATCH] ls-refs: reuse buffer when sending refs

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 25, 2021, 13:49 UTC
Message-ID: <ccd03e685af0f5cf25c68272a758fc88d115e37a.1629899211.git.ps@pks.im>

In the initial reference advertisement, the Git server will first announce all of its references to the client. The logic is handled in `send_ref()`, which will allocate a new buffer for each refline it is about to send. This is quite wasteful: instead of allocating a new buffer each time, we can just reuse a buffer.

Improve this by passing in a buffer via the `ls_refs_data` struct which is then reused on each reference. In a repository with about 2.3M refs, this speeds up local mirror fetches by about 2%:

    Benchmark #1: HEAD~: git-fetch
      Time (mean ± σ):     25.415 s ±  0.131 s    [User: 22.722 s, System: 4.740 s]
      Range (min … max):   25.240 s … 25.543 s    5 runs

    Benchmark #2: HEAD: git-fetch
      Time (mean ± σ):     24.922 s ±  0.110 s    [User: 22.404 s, System: 4.476 s]
      Range (min … max):   24.825 s … 25.081 s    5 runs

    Summary
      'HEAD: git-fetch' ran
        1.02 ± 0.01 times faster than 'HEAD~: git-fetch'

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

Note that while this topic applies on top of "master", I've done the benchmark on top of my other optimizations for fetches. It's cheating a bit, but it's easier to see that the optimization does something when the remaining constant part is lower.

 ls-refs.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/ls-refs.c b/ls-refs.c
index 88f6c3f60d..84021416ca 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -65,6 +65,7 @@ struct ls_refs_data {
 	unsigned peel;
 	unsigned symrefs;
 	struct strvec prefixes;
+	struct strbuf buf;
 	unsigned unborn : 1;
 };
 
@@ -73,7 +74,8 @@ static int send_ref(const char *refname, const struct object_id *oid,
 {
 	struct ls_refs_data *data = cb_data;
 	const char *refname_nons = strip_namespace(refname);
-	struct strbuf refline = STRBUF_INIT;
+
+	strbuf_reset(&data->buf);
 
 	if (ref_is_hidden(refname_nons, refname))
 		return 0;
@@ -82,9 +84,9 @@ static int send_ref(const char *refname, const struct object_id *oid,
 		return 0;
 
 	if (oid)
-		strbuf_addf(&refline, "%s %s", oid_to_hex(oid), refname_nons);
+		strbuf_addf(&data->buf, "%s %s", oid_to_hex(oid), refname_nons);
 	else
-		strbuf_addf(&refline, "unborn %s", refname_nons);
+		strbuf_addf(&data->buf, "unborn %s", refname_nons);
 	if (data->symrefs && flag & REF_ISSYMREF) {
 		struct object_id unused;
 		const char *symref_target = resolve_ref_unsafe(refname, 0,
@@ -94,20 +96,19 @@ static int send_ref(const char *refname, const struct object_id *oid,
 		if (!symref_target)
 			die("'%s' is a symref but it is not?", refname);
 
-		strbuf_addf(&refline, " symref-target:%s",
+		strbuf_addf(&data->buf, " symref-target:%s",
 			    strip_namespace(symref_target));
 	}
 
 	if (data->peel && oid) {
 		struct object_id peeled;
 		if (!peel_iterated_oid(oid, &peeled))
-			strbuf_addf(&refline, " peeled:%s", oid_to_hex(&peeled));
+			strbuf_addf(&data->buf, " peeled:%s", oid_to_hex(&peeled));
 	}
 
-	strbuf_addch(&refline, '\n');
-	packet_write(1, refline.buf, refline.len);
+	strbuf_addch(&data->buf, '\n');
+	packet_write(1, data->buf.buf, data->buf.len);
 
-	strbuf_release(&refline);
 	return 0;
 }
 
@@ -145,6 +146,7 @@ int ls_refs(struct repository *r, struct strvec *keys,
 
 	memset(&data, 0, sizeof(data));
 	strvec_init(&data.prefixes);
+	strbuf_init(&data.buf, 0);
 
 	ensure_config_read();
 	git_config(ls_refs_config, NULL);
@@ -173,6 +175,7 @@ int ls_refs(struct repository *r, struct strvec *keys,
 				     send_ref, &data, 0);
 	packet_flush(1);
 	strvec_clear(&data.prefixes);
+	strbuf_release(&data.buf);
 	return 0;
 }

-- 
2.33.0