upload-pack: use stdio in send_ref callbacks

In both protocol v0 and v2, upload-pack writes one pktline packet per
advertised ref to stdout. That means one or two write(2) syscalls per
ref. This is problematic if these writes become network sends with
high overhead.

This commit changes both send_ref callbacks to use buffered IO using
stdio.

To give an example of the impact: I set up a single-threaded loop that
calls ls-remote (with HTTP and protocol v2) on a local GitLab
instance, on a repository with 11K refs. When I switch from Git
v2.32.0 to this patch, I see a 40% reduction in CPU time for Git, and
65% for Gitaly (GitLab's Git RPC service).

So using buffered IO not only saves syscalls in upload-pack, it also
saves time in things that consume upload-pack's output.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Jacob Vosmaer <jacob@gitlab.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/ls-refs.c b/ls-refs.c
index 88f6c3f60d8ef61373a1bfed33f49c877b6af320..e6a2dbd96230d246560cfcafb37aba0b6bd080cc 100644 (file)
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -105,7 +105,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
        }
 
        strbuf_addch(&refline, '\n');
-       packet_write(1, refline.buf, refline.len);
+       packet_fwrite(stdout, refline.buf, refline.len);
 
        strbuf_release(&refline);
        return 0;
@@ -171,7 +171,7 @@ int ls_refs(struct repository *r, struct strvec *keys,
                strvec_push(&data.prefixes, "");
        for_each_fullref_in_prefixes(get_git_namespace(), data.prefixes.v,
                                     send_ref, &data, 0);
-       packet_flush(1);
+       packet_fflush(stdout);
        strvec_clear(&data.prefixes);
        return 0;
 }

diff --git a/upload-pack.c b/upload-pack.c
index 297b76fcb436794c8458bfaf5689e6fa30cae556..0ed377b1fb298a5272d77803c4a0155f938904b4 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1207,7 +1207,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
 
                format_symref_info(&symref_info, &data->symref);
                format_session_id(&session_id, data);
-               packet_write_fmt(1, "%s %s%c%s%s%s%s%s%s%s object-format=%s agent=%s\n",
+               packet_fwrite_fmt(stdout, "%s %s%c%s%s%s%s%s%s%s object-format=%s agent=%s\n",
                             oid_to_hex(oid), refname_nons,
                             0, capabilities,
                             (data->allow_uor & ALLOW_TIP_SHA1) ?
@@ -1223,11 +1223,11 @@ static int send_ref(const char *refname, const struct object_id *oid,
                strbuf_release(&symref_info);
                strbuf_release(&session_id);
        } else {
-               packet_write_fmt(1, "%s %s\n", oid_to_hex(oid), refname_nons);
+               packet_fwrite_fmt(stdout, "%s %s\n", oid_to_hex(oid), refname_nons);
        }
        capabilities = NULL;
        if (!peel_iterated_oid(oid, &peeled))
-               packet_write_fmt(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
+               packet_fwrite_fmt(stdout, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
        return 0;
 }
 
@@ -1348,6 +1348,11 @@ void upload_pack(struct upload_pack_options *options)
                reset_timeout(data.timeout);
                head_ref_namespaced(send_ref, &data);
                for_each_namespaced_ref(send_ref, &data);
+               /*
+                * fflush stdout before calling advertise_shallow_grafts because send_ref
+                * uses stdio.
+                */
+               fflush_or_die(stdout);
                advertise_shallow_grafts(1);
                packet_flush(1);
        } else {