@matheusbsbs / matheus-bagliano / commits / 55a9651d26

upload-pack.c: increase output buffer size

When serving a fetch, git upload-pack copies data from a git pack-objects stdout pipe to its stdout. This commit increases the size of the buffer used for that copying from 8192 to 65515, the maximum sideband-64k packet size. Previously, this buffer was allocated on the stack. Because the new buffer size is nearly 64KB, we switch this to a heap allocation. On GitLab.com we use GitLab's pack-objects cache which does writes of 65515 bytes. Because of the default 8KB buffer size, propagating these cache writes requires 8 pipe reads and 8 pipe writes from git-upload-pack, and 8 pipe reads from Gitaly (our Git RPC service). If we increase the size of the buffer to the maximum Git packet size, we need only 1 pipe read and 1 pipe write in git-upload-pack, and 1 pipe read in Gitaly to transfer the same amount of data. In benchmarks with a pure fetch and 100% cache hit rate workload we are seeing CPU utilization reductions of over 30%. Signed-off-by: Jacob Vosmaer <jacob@gitlab.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jacob Vosmaer committed Dec 14, 2021 at 20:46 UTC 55a9651d26a6b88c68445e7d6c9f511d1207cbd8

1 file changed +12 -5

upload-pack.c

+12 -5

@@ -194,7 +194,13 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)

194 }

195

196 struct output_state {

197 - char buffer[8193];

197 + /*

198 + * We do writes no bigger than LARGE_PACKET_DATA_MAX - 1, because with

199 + * sideband-64k the band designator takes up 1 byte of space. Because

200 + * relay_pack_data keeps the last byte to itself, we make the buffer 1

201 + * byte bigger than the intended maximum write size.

202 + */

203 + char buffer[(LARGE_PACKET_DATA_MAX - 1) + 1];

204 int used;

205 unsigned packfile_uris_started : 1;

206 unsigned packfile_started : 1;

@@ -269,7 +275,7 @@ static void create_pack_file(struct upload_pack_data *pack_data,

275 const struct string_list *uri_protocols)

276 {

277 struct child_process pack_objects = CHILD_PROCESS_INIT;

272 - struct output_state output_state = { { 0 } };

278 + struct output_state *output_state = xcalloc(1, sizeof(struct output_state));

279 char progress[128];

280 char abort_msg[] = "aborting due to possible repository "

281 "corruption on the remote side.";

@@ -404,7 +410,7 @@ static void create_pack_file(struct upload_pack_data *pack_data,

410 }

411 if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {

412 int result = relay_pack_data(pack_objects.out,

407 - &output_state,

413 + output_state,

414 pack_data->use_sideband,

415 !!uri_protocols);

416

@@ -438,11 +444,12 @@ static void create_pack_file(struct upload_pack_data *pack_data,

444 }

445

446 /* flush the data */

441 - if (output_state.used > 0) {

442 - send_client_data(1, output_state.buffer, output_state.used,

447 + if (output_state->used > 0) {

448 + send_client_data(1, output_state->buffer, output_state->used,

449 pack_data->use_sideband);

450 fprintf(stderr, "flushed.\n");

451 }

452 + free(output_state);

453 if (pack_data->use_sideband)

454 packet_flush(1);

455 return;