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>

Jacob Vosmaer committed Sep 1, 2021 at 14:54 UTC 70afef5cdf29b5159f18df1b93722055f78740f8
2 files changed +10 -5
ls-refs.c
+2 -2
@@ -105,7 +105,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
105 }
106
107 strbuf_addch(&refline, '\n');
108 - packet_write(1, refline.buf, refline.len);
108 + packet_fwrite(stdout, refline.buf, refline.len);
109
110 strbuf_release(&refline);
111 return 0;
@@ -171,7 +171,7 @@ int ls_refs(struct repository *r, struct strvec *keys,
171 strvec_push(&data.prefixes, "");
172 for_each_fullref_in_prefixes(get_git_namespace(), data.prefixes.v,
173 send_ref, &data, 0);
174 - packet_flush(1);
174 + packet_fflush(stdout);
175 strvec_clear(&data.prefixes);
176 return 0;
177 }
upload-pack.c
+8 -3
@@ -1207,7 +1207,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
1207
1208 format_symref_info(&symref_info, &data->symref);
1209 format_session_id(&session_id, data);
1210 - packet_write_fmt(1, "%s %s%c%s%s%s%s%s%s%s object-format=%s agent=%s\n",
1210 + packet_fwrite_fmt(stdout, "%s %s%c%s%s%s%s%s%s%s object-format=%s agent=%s\n",
1211 oid_to_hex(oid), refname_nons,
1212 0, capabilities,
1213 (data->allow_uor & ALLOW_TIP_SHA1) ?
@@ -1223,11 +1223,11 @@ static int send_ref(const char *refname, const struct object_id *oid,
1223 strbuf_release(&symref_info);
1224 strbuf_release(&session_id);
1225 } else {
1226 - packet_write_fmt(1, "%s %s\n", oid_to_hex(oid), refname_nons);
1226 + packet_fwrite_fmt(stdout, "%s %s\n", oid_to_hex(oid), refname_nons);
1227 }
1228 capabilities = NULL;
1229 if (!peel_iterated_oid(oid, &peeled))
1230 - packet_write_fmt(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
1230 + packet_fwrite_fmt(stdout, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
1231 return 0;
1232 }
1233
@@ -1348,6 +1348,11 @@ void upload_pack(struct upload_pack_options *options)
1348 reset_timeout(data.timeout);
1349 head_ref_namespaced(send_ref, &data);
1350 for_each_namespaced_ref(send_ref, &data);
1351 + /*
1352 + * fflush stdout before calling advertise_shallow_grafts because send_ref
1353 + * uses stdio.
1354 + */
1355 + fflush_or_die(stdout);
1356 advertise_shallow_grafts(1);
1357 packet_flush(1);
1358 } else {