upload-pack: prefer flushing data over sending keepalive

When using the sideband in git-upload-pack(1) we know to send out
keepalive packets in case generating the pack takes too long. These
keepalives take the form of a simple empty pktline.

In the preceding commit we have adapted git-upload-pack(1) to buffer
data more aggressively before sending it to the client. This creates an
obvious optimization opportunity: when we hit the keepalive timeout
while we still hold on to some buffered data, then it makes more sense
to flush out the data instead of sending the empty keepalive packet.

This is overall not going to be a significant win. Most keepalives will
come before the pack data starts, and once pack-objects starts producing
data, it tends to do so pretty consistently. And of course we can't send
data before we see the PACK header, because the whole point is to buffer
the early bit waiting for packfile URIs. But the optimization is easy
enough to realize.

Do so and flush out data instead of sending an empty pktline.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/upload-pack.c b/upload-pack.c
index 04521e57c967f302af77735aab8db6b34109d8bb..ef8f8189c1714f493841cced77a867ec53c7c431 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -466,18 +466,27 @@ static void create_pack_file(struct upload_pack_data *pack_data,
                }
 
                /*
-                * We hit the keepalive timeout without saying anything; send
-                * an empty message on the data sideband just to let the other
-                * side know we're still working on it, but don't have any data
-                * yet.
+                * We hit the keepalive timeout without saying anything. If we
+                * have pending data we flush it out to the caller now.
+                * Otherwise, we send an empty message on the data sideband
+                * just to let the other side know we're still working on it,
+                * but don't have any data yet.
                 *
                 * If we don't have a sideband channel, there's no room in the
                 * protocol to say anything, so those clients are just out of
                 * luck.
                 */
                if (!ret && pack_data->use_sideband) {
-                       static const char buf[] = "0005\1";
-                       write_or_die(1, buf, 5);
+                       if (output_state->packfile_started && output_state->used > 1) {
+                               send_client_data(1, output_state->buffer, output_state->used - 1,
+                                                pack_data->use_sideband);
+                               output_state->buffer[0] = output_state->buffer[output_state->used - 1];
+                               output_state->used = 1;
+                       } else {
+                               static const char buf[] = "0005\1";
+                               write_or_die(1, buf, 5);
+                       }
+
                        last_sent_ms = now_ms;
                }
        }