{"thread":{"id":"65225","subject":"[PATCH v4 01/10] upload-pack: fix debug statement when flushing packfile data","startedAt":"2026-03-13T06:45:22Z","lastAt":"2026-03-13T06:45:46Z","messageCount":11,"participants":["Patrick Steinhardt"],"isPatch":true,"patchVersion":4,"patchTotal":10},"messages":[{"id":"538854","messageId":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im","subject":"[PATCH v4 00/10] upload-pack: reduce lock contention when writing packfile data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:11Z","receivedAt":"2026-03-13T06:45:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes some heavy lock contention when writing\ndata from git-upload-pack(1) into pipes. This lock contention can be\nobserved when having hundreds of git-upload-pack(1) processes active at\nthe same time that write data into pipes at dozens of gigabits per\nsecond.\n\nI have uploaded the flame graph that clearly shows the lock contention\nat [1].\n\nChanges in v4:\n  - Drop a stale half-sentence that I wanted to remove.\n  - Link to v3: https://lore.kernel.org/r/20260310-pks-upload-pack-write-contention-v3-0-8bc97aa3e267@pks.im\n\nChanges in v3:\n  - Fix handling of `iov_len` overflows in writev(3p) wrapper.\n  - Add another patch that causes us to flush out data instead of\n    sending a 0005 keepalive packet.\n  - Link to v2: https://lore.kernel.org/r/20260303-pks-upload-pack-write-contention-v2-0-7321830f08fe@pks.im\n\nChanges in v2:\n  - Change the buffer size in git-pack-objects(1) to also reduce the\n    number of write syscalls over there.\n  - Introduce writev to half the number of syscalls when writing\n    pktlines.\n  - Use `sizeof(os->buffer)` instead of open-coding its size.\n  - Improve keepalive logic in git-upload-pack(1) to account for\n    buffering.\n  - Link to v1: https://lore.kernel.org/r/20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im\n\nThanks!\n\nPatrick\n\n[1]: https://gitlab.com/gitlab-org/git/-/work_items/675\n\n---\nPatrick Steinhardt (10):\n      upload-pack: fix debug statement when flushing packfile data\n      upload-pack: adapt keepalives based on buffering\n      upload-pack: prefer flushing data over sending keepalive\n      upload-pack: reduce lock contention when writing packfile data\n      compat/posix: introduce writev(3p) wrapper\n      wrapper: introduce writev(3p) wrappers\n      sideband: use writev(3p) to send pktlines\n      csum-file: introduce `hashfd_ext()`\n      csum-file: drop `hashfd_throughput()`\n      builtin/pack-objects: reduce lock contention when writing packfile data\n\n Makefile               |  4 +++\n builtin/pack-objects.c | 23 +++++++++++---\n compat/posix.h         | 14 +++++++++\n compat/writev.c        | 44 +++++++++++++++++++++++++++\n config.mak.uname       |  2 ++\n csum-file.c            | 28 +++++------------\n csum-file.h            | 16 ++++++++--\n meson.build            |  1 +\n sideband.c             | 14 +++++++--\n upload-pack.c          | 81 +++++++++++++++++++++++++++++++++++++++-----------\n wrapper.c              | 41 +++++++++++++++++++++++++\n wrapper.h              |  9 ++++++\n write-or-die.c         |  8 +++++\n write-or-die.h         |  1 +\n 14 files changed, 239 insertions(+), 47 deletions(-)\n\nRange-diff versus v3:\n\n 1:  65471f969b =  1:  f9896a8451 upload-pack: fix debug statement when flushing packfile data\n 2:  c5705c1cb1 =  2:  b1bc6f5749 upload-pack: adapt keepalives based on buffering\n 3:  f34fa584f4 !  3:  fcf5f06375 upload-pack: prefer flushing data over sending keepalive\n    @@ Commit message\n         the early bit waiting for packfile URIs. But the optimization is easy\n         enough to realize.\n     \n    -    Do so and flush out data instead of sending an empty pktline. While at\n    -    it, drop the useless\n    +    Do so and flush out data instead of sending an empty pktline.\n     \n         Suggested-by: Jeff King <peff@peff.net>\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n 4:  8d08e93929 =  4:  f2bb7a38aa upload-pack: reduce lock contention when writing packfile data\n 5:  aa42217e43 =  5:  78a1bbb810 compat/posix: introduce writev(3p) wrapper\n 6:  c5d194ca2b =  6:  c199dd398a wrapper: introduce writev(3p) wrappers\n 7:  e4df805840 =  7:  b7fe2b818f sideband: use writev(3p) to send pktlines\n 8:  16f9968bcd =  8:  b9d33b93cd csum-file: introduce `hashfd_ext()`\n 9:  47ae58440b =  9:  b2af62c4ff csum-file: drop `hashfd_throughput()`\n10:  c92ccf9df2 = 10:  47f5090b66 builtin/pack-objects: reduce lock contention when writing packfile data\n\n---\nbase-commit: fb1b83bcddff60463f6e86bb021784c88d0b748c\nchange-id: 20260227-pks-upload-pack-write-contention-435ce01f5fe9\n\n"},{"id":"538853","messageId":"20260313-pks-upload-pack-write-contention-v4-1-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 01/10] upload-pack: fix debug statement when flushing packfile data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:12Z","receivedAt":"2026-03-13T06:45:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When git-upload-pack(1) writes packfile data to the client we have some\nlogic in place that buffers some partial lines. When that buffer still\ncontains data after git-pack-objects(1) has finished we flush the buffer\nso that all remaining bytes are sent out.\n\nCuriously, when we do so we also print the string \"flushed.\" to stderr.\nThis statement has been introduced in b1c71b7281 (upload-pack: avoid\nsending an incomplete pack upon failure, 2006-06-20), so quite a while\nago. What's interesting though is that stderr is typically spliced\nthrough to the client-side, and consequently the client would see this\nmessage. Munging the way how we do the caching indeed confirms this:\n\n  $ git clone file:///home/pks/Development/linux/\n  Cloning into bare repository 'linux.git'...\n  remote: Enumerating objects: 12980346, done.\n  remote: Counting objects: 100% (131820/131820), done.\n  remote: Compressing objects: 100% (50290/50290), done.\n  remote: Total 12980346 (delta 96319), reused 104500 (delta 81217), pack-reused 12848526 (from 1)\n  Receiving objects: 100% (12980346/12980346), 3.23 GiB | 57.44 MiB/s, done.\n  flushed.\n  Resolving deltas: 100% (10676718/10676718), done.\n\nIt's quite clear that this string shouldn't ever be visible to the\nclient, so it rather feels like this is a left-over debug statement. The\nmenitoned commit doesn't mention this line, either.\n\nRemove the debug output to prepare for a change in how we do the\nbuffering in the next commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n upload-pack.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex e8c5cce1c7..b3a8561ef5 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -457,11 +457,9 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t}\n \n \t/* flush the data */\n-\tif (output_state->used > 0) {\n+\tif (output_state->used > 0)\n \t\tsend_client_data(1, output_state->buffer, output_state->used,\n \t\t\t\t pack_data->use_sideband);\n-\t\tfprintf(stderr, \"flushed.\\n\");\n-\t}\n \tfree(output_state);\n \tif (pack_data->use_sideband)\n \t\tpacket_flush(1);\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538855","messageId":"20260313-pks-upload-pack-write-contention-v4-2-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 02/10] upload-pack: adapt keepalives based on buffering","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:13Z","receivedAt":"2026-03-13T06:45:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `create_pack_file()` is responsible for sending the\npackfile data to the client of git-upload-pack(1). As generating the\nbytes may take significant computing resources we also have a mechanism\nin place that optionally sends keepalive pktlines in case we haven't\nsent out any data.\n\nThe keepalive logic is purely based poll(3p): we pass a timeout to that\nsyscall, and if the call times out we send out the keepalive pktline.\nWhile reasonable, this logic isn't entirely sufficient: even if the call\nto poll(3p) ends because we have received data on any of the file\ndescriptors we may not necessarily send data to the client.\n\nThe most important edge case here happens in `relay_pack_data()`. When\nwe haven't seen the initial \"PACK\" signature from git-pack-objects(1)\nyet we buffer incoming data. So in the worst case, if each of the bytes\nof that signature arrive shortly before the configured keepalive\ntimeout, then we may not send out any data for a time period that is\n(almost) four times as long as the configured timeout.\n\nThis edge case is rather unlikely to matter in practice. But in a\nsubsequent commit we're going to adapt our buffering mechanism to become\nmore aggressive, which makes it more likely that we don't send any data\nfor an extended amount of time.\n\nAdapt the logic so that instead of using a fixed timeout on every call\nto poll(3p), we instead figure out how much time has passed since the\nlast-sent data.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n upload-pack.c | 49 ++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 40 insertions(+), 9 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b3a8561ef5..f6f380a601 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -29,6 +29,7 @@\n #include \"commit-graph.h\"\n #include \"commit-reach.h\"\n #include \"shallow.h\"\n+#include \"trace.h\"\n #include \"write-or-die.h\"\n #include \"json-writer.h\"\n #include \"strmap.h\"\n@@ -218,7 +219,8 @@ struct output_state {\n };\n \n static int relay_pack_data(int pack_objects_out, struct output_state *os,\n-\t\t\t   int use_sideband, int write_packfile_line)\n+\t\t\t   int use_sideband, int write_packfile_line,\n+\t\t\t   bool *did_send_data)\n {\n \t/*\n \t * We keep the last byte to ourselves\n@@ -232,6 +234,8 @@ static int relay_pack_data(int pack_objects_out, struct output_state *os,\n \t */\n \tssize_t readsz;\n \n+\t*did_send_data = false;\n+\n \treadsz = xread(pack_objects_out, os->buffer + os->used,\n \t\t       sizeof(os->buffer) - os->used);\n \tif (readsz < 0) {\n@@ -247,6 +251,7 @@ static int relay_pack_data(int pack_objects_out, struct output_state *os,\n \t\t\t\tif (os->packfile_uris_started)\n \t\t\t\t\tpacket_delim(1);\n \t\t\t\tpacket_write_fmt(1, \"\\1packfile\\n\");\n+\t\t\t\t*did_send_data = true;\n \t\t\t}\n \t\t\tbreak;\n \t\t}\n@@ -259,6 +264,7 @@ static int relay_pack_data(int pack_objects_out, struct output_state *os,\n \t\t\t}\n \t\t\t*p = '\\0';\n \t\t\tpacket_write_fmt(1, \"\\1%s\\n\", os->buffer);\n+\t\t\t*did_send_data = true;\n \n \t\t\tos->used -= p - os->buffer + 1;\n \t\t\tmemmove(os->buffer, p + 1, os->used);\n@@ -279,6 +285,7 @@ static int relay_pack_data(int pack_objects_out, struct output_state *os,\n \t\tos->used = 0;\n \t}\n \n+\t*did_send_data = true;\n \treturn readsz;\n }\n \n@@ -290,6 +297,7 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \tchar progress[128];\n \tchar abort_msg[] = \"aborting due to possible repository \"\n \t\t\"corruption on the remote side.\";\n+\tuint64_t last_sent_ms = 0;\n \tssize_t sz;\n \tint i;\n \tFILE *pipe_fd;\n@@ -365,10 +373,14 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t */\n \n \twhile (1) {\n+\t\tuint64_t now_ms = getnanotime() / 1000000;\n \t\tstruct pollfd pfd[2];\n-\t\tint pe, pu, pollsize, polltimeout;\n+\t\tint pe, pu, pollsize, polltimeout_ms;\n \t\tint ret;\n \n+\t\tif (!last_sent_ms)\n+\t\t\tlast_sent_ms = now_ms;\n+\n \t\treset_timeout(pack_data->timeout);\n \n \t\tpollsize = 0;\n@@ -390,11 +402,21 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t\tif (!pollsize)\n \t\t\tbreak;\n \n-\t\tpolltimeout = pack_data->keepalive < 0\n-\t\t\t? -1\n-\t\t\t: 1000 * pack_data->keepalive;\n+\t\tif (pack_data->keepalive < 0) {\n+\t\t\tpolltimeout_ms = -1;\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * The polling timeout needs to be adjusted based on\n+\t\t\t * the time we have sent our last package. The longer\n+\t\t\t * it's been in the past, the shorter the timeout\n+\t\t\t * becomes until we eventually don't block at all.\n+\t\t\t */\n+\t\t\tpolltimeout_ms = 1000 * pack_data->keepalive - (now_ms - last_sent_ms);\n+\t\t\tif (polltimeout_ms < 0)\n+\t\t\t\tpolltimeout_ms = 0;\n+\t\t}\n \n-\t\tret = poll(pfd, pollsize, polltimeout);\n+\t\tret = poll(pfd, pollsize, polltimeout_ms);\n \n \t\tif (ret < 0) {\n \t\t\tif (errno != EINTR) {\n@@ -403,16 +425,18 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n+\n \t\tif (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) {\n \t\t\t/* Status ready; we ship that in the side-band\n \t\t\t * or dump to the standard error.\n \t\t\t */\n \t\t\tsz = xread(pack_objects.err, progress,\n \t\t\t\t  sizeof(progress));\n-\t\t\tif (0 < sz)\n+\t\t\tif (0 < sz) {\n \t\t\t\tsend_client_data(2, progress, sz,\n \t\t\t\t\t\t pack_data->use_sideband);\n-\t\t\telse if (sz == 0) {\n+\t\t\t\tlast_sent_ms = now_ms;\n+\t\t\t} else if (sz == 0) {\n \t\t\t\tclose(pack_objects.err);\n \t\t\t\tpack_objects.err = -1;\n \t\t\t}\n@@ -421,11 +445,14 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t\t\t/* give priority to status messages */\n \t\t\tcontinue;\n \t\t}\n+\n \t\tif (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {\n+\t\t\tbool did_send_data;\n \t\t\tint result = relay_pack_data(pack_objects.out,\n \t\t\t\t\t\t     output_state,\n \t\t\t\t\t\t     pack_data->use_sideband,\n-\t\t\t\t\t\t     !!uri_protocols);\n+\t\t\t\t\t\t     !!uri_protocols,\n+\t\t\t\t\t\t     &did_send_data);\n \n \t\t\tif (result == 0) {\n \t\t\t\tclose(pack_objects.out);\n@@ -433,6 +460,9 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t\t\t} else if (result < 0) {\n \t\t\t\tgoto fail;\n \t\t\t}\n+\n+\t\t\tif (did_send_data)\n+\t\t\t\tlast_sent_ms = now_ms;\n \t\t}\n \n \t\t/*\n@@ -448,6 +478,7 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t\tif (!ret && pack_data->use_sideband) {\n \t\t\tstatic const char buf[] = \"0005\\1\";\n \t\t\twrite_or_die(1, buf, 5);\n+\t\t\tlast_sent_ms = now_ms;\n \t\t}\n \t}\n \n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538856","messageId":"20260313-pks-upload-pack-write-contention-v4-3-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 03/10] upload-pack: prefer flushing data over sending keepalive","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:14Z","receivedAt":"2026-03-13T06:45:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When using the sideband in git-upload-pack(1) we know to send out\nkeepalive packets in case generating the pack takes too long. These\nkeepalives take the form of a simple empty pktline.\n\nIn the preceding commit we have adapted git-upload-pack(1) to buffer\ndata more aggressively before sending it to the client. This creates an\nobvious optimization opportunity: when we hit the keepalive timeout\nwhile we still hold on to some buffered data, then it makes more sense\nto flush out the data instead of sending the empty keepalive packet.\n\nThis is overall not going to be a significant win. Most keepalives will\ncome before the pack data starts, and once pack-objects starts producing\ndata, it tends to do so pretty consistently. And of course we can't send\ndata before we see the PACK header, because the whole point is to buffer\nthe early bit waiting for packfile URIs. But the optimization is easy\nenough to realize.\n\nDo so and flush out data instead of sending an empty pktline.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n upload-pack.c | 21 +++++++++++++++------\n 1 file changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex f6f380a601..7a165d226d 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -466,18 +466,27 @@ static void create_pack_file(struct upload_pack_data *pack_data,\n \t\t}\n \n \t\t/*\n-\t\t * We hit the keepalive timeout without saying anything; send\n-\t\t * an empty message on the data sideband just to let the other\n-\t\t * side know we're still working on it, but don't have any data\n-\t\t * yet.\n+\t\t * We hit the keepalive timeout without saying anything. If we\n+\t\t * have pending data we flush it out to the caller now.\n+\t\t * Otherwise, we send an empty message on the data sideband\n+\t\t * just to let the other side know we're still working on it,\n+\t\t * but don't have any data yet.\n \t\t *\n \t\t * If we don't have a sideband channel, there's no room in the\n \t\t * protocol to say anything, so those clients are just out of\n \t\t * luck.\n \t\t */\n \t\tif (!ret && pack_data->use_sideband) {\n-\t\t\tstatic const char buf[] = \"0005\\1\";\n-\t\t\twrite_or_die(1, buf, 5);\n+\t\t\tif (output_state->packfile_started && output_state->used > 1) {\n+\t\t\t\tsend_client_data(1, output_state->buffer, output_state->used - 1,\n+\t\t\t\t\t\t pack_data->use_sideband);\n+\t\t\t\toutput_state->buffer[0] = output_state->buffer[output_state->used - 1];\n+\t\t\t\toutput_state->used = 1;\n+\t\t\t} else {\n+\t\t\t\tstatic const char buf[] = \"0005\\1\";\n+\t\t\t\twrite_or_die(1, buf, 5);\n+\t\t\t}\n+\n \t\t\tlast_sent_ms = now_ms;\n \t\t}\n \t}\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538857","messageId":"20260313-pks-upload-pack-write-contention-v4-4-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 04/10] upload-pack: reduce lock contention when writing packfile data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:15Z","receivedAt":"2026-03-13T06:45:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In our production systems we have recently observed write contention in\ngit-upload-pack(1). The system in question was consistently streaming\npackfiles at a rate of dozens of gigabits per second, but curiously the\nsystem was neither bottlenecked on CPU, memory or IOPS.\n\nWe eventually discovered that Git was spending 80% of its time in\n`pipe_write()`, out of which almost all of the time was spent in the\n`ep_poll_callback` function in the kernel. Quoting the reporter:\n\n  This infrastructure is part of an event notification queue designed to\n  allow for multiple producers to emit events, but that concurrency\n  safety is guarded by 3 layers of locking. The layer we're hitting\n  contention in uses a simple reader/writer lock mode (a.k.a. shared\n  versus exclusive mode), where producers need shared-mode (read mode),\n  and various other actions use exclusive (write) mode.\n\nThe system in question generates workloads where we have hundreds of\ngit-upload-pack(1) processes active at the same point in time. These\nprocesses end up contending around those locks, and the consequence is\nthat the Git processes stall.\n\nNow git-upload-pack(1) already has the infrastructure in place to buffer\nsome of the data it reads from git-pack-objects(1) before actually\nsending it out. We only use this infrastructure in very limited ways\nthough, so we generally end up matching one read(3p) call with one\nwrite(3p) call. Even worse, when the sideband is enabled we end up\nmatching one read with _two_ writes: one for the pkt-line length, and\none for the packfile data.\n\nExtend our use of the buffering infrastructure so that we soak up bytes\nuntil the buffer is filled up at least 2/3rds of its capacity. The\nchange is relatively simple to implement as we already know to flush the\nbuffer in `create_pack_file()` after git-pack-objects(1) has finished.\n\nThis significantly reduces the number of write(3p) syscalls we need to\ndo. Before this change, cloning the Linux repository resulted in around\n400,000 write(3p) syscalls. With the buffering in place we only do\naround 130,000 syscalls.\n\nNow we could of course go even further and make sure that we always fill\nup the whole buffer. But this might cause an increase in read(3p)\nsyscalls, and some tests show that this only reduces the number of\nwrite(3p) syscalls from 130,000 to 100,000. So overall this doesn't seem\nworth it.\n\nNote that the issue could also be fixed by adapting the write buffer\nthat we use in the downstream git-pack-objects(1) command, and such a\nchange would have roughly the same result. But the command that\ngenerates the packfile data may not always be git-pack-objects(1) as it\ncan be changed via \"uploadpack.packObjectsHook\", so such a fix would\nonly help in _some_ cases. Regardless of that, we'll also adapt the\nwrite buffer size of git-pack-objects(1) in a subsequent commit.\n\nHelped-by: Matt Smiley <msmiley@gitlab.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n upload-pack.c | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 7a165d226d..9f6d6fe48c 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -276,6 +276,13 @@ static int relay_pack_data(int pack_objects_out, struct output_state *os,\n \t\t}\n \t}\n \n+\t/*\n+\t * Make sure that we buffer some data before sending it to the client.\n+\t * This significantly reduces the number of write(3p) syscalls.\n+\t */\n+\tif (readsz && os->used < (sizeof(os->buffer) * 2 / 3))\n+\t\treturn readsz;\n+\n \tif (os->used > 1) {\n \t\tsend_client_data(1, os->buffer, os->used - 1, use_sideband);\n \t\tos->buffer[0] = os->buffer[os->used - 1];\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538858","messageId":"20260313-pks-upload-pack-write-contention-v4-5-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 05/10] compat/posix: introduce writev(3p) wrapper","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:16Z","receivedAt":"2026-03-13T06:45:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a subsequent commit we're going to add the first caller to\nwritev(3p). Introduce a compatibility wrapper for this syscall that we\ncan use on systems that don't have this syscall.\n\nThe syscall exists on modern Unixes like Linux and macOS, and seemingly\neven for NonStop according to [1]. It doesn't seem to exist on Windows\nthough.\n\n[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf\n[2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile         |  4 ++++\n compat/posix.h   | 14 ++++++++++++++\n compat/writev.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++\n config.mak.uname |  2 ++\n meson.build      |  1 +\n 5 files changed, 65 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex f3264d0a37..493851162d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2021,6 +2021,10 @@ ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\n \tCOMPAT_OBJS += compat/pread.o\n endif\n+ifdef NO_WRITEV\n+\tCOMPAT_CFLAGS += -DNO_WRITEV\n+\tCOMPAT_OBJS += compat/writev.o\n+endif\n ifdef NO_FAST_WORKING_DIRECTORY\n \tBASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY\n endif\ndiff --git a/compat/posix.h b/compat/posix.h\nindex 245386fa4a..3c611d2736 100644\n--- a/compat/posix.h\n+++ b/compat/posix.h\n@@ -137,6 +137,9 @@\n #include <sys/socket.h>\n #include <sys/ioctl.h>\n #include <sys/statvfs.h>\n+#ifndef NO_WRITEV\n+#include <sys/uio.h>\n+#endif\n #include <termios.h>\n #ifndef NO_SYS_SELECT_H\n #include <sys/select.h>\n@@ -323,6 +326,17 @@ int git_lstat(const char *, struct stat *);\n ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);\n #endif\n \n+#ifdef NO_WRITEV\n+#define writev git_writev\n+#define iovec git_iovec\n+struct git_iovec {\n+\tvoid *iov_base;\n+\tsize_t iov_len;\n+};\n+\n+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt);\n+#endif\n+\n #ifdef NO_SETENV\n #define setenv gitsetenv\n int gitsetenv(const char *, const char *, int);\ndiff --git a/compat/writev.c b/compat/writev.c\nnew file mode 100644\nindex 0000000000..3a94870a2f\n--- /dev/null\n+++ b/compat/writev.c\n@@ -0,0 +1,44 @@\n+#include \"../git-compat-util.h\"\n+#include \"../wrapper.h\"\n+\n+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)\n+{\n+\tsize_t total_written = 0;\n+\tsize_t sum = 0;\n+\n+\t/*\n+\t * According to writev(3p), the syscall shall error with EINVAL in case\n+\t * the sum of `iov_len` overflows `ssize_t`.\n+\t */\n+\t for (int i = 0; i < iovcnt; i++) {\n+\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n+\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n+\t\t\terrno = EINVAL;\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tsum += iov[i].iov_len;\n+\t}\n+\n+\tfor (int i = 0; i < iovcnt; i++) {\n+\t\tconst char *bytes = iov[i].iov_base;\n+\t\tsize_t iovec_written = 0;\n+\n+\t\twhile (iovec_written < iov[i].iov_len) {\n+\t\t\tssize_t bytes_written = xwrite(fd, bytes + iovec_written,\n+\t\t\t\t\t\t       iov[i].iov_len - iovec_written);\n+\t\t\tif (bytes_written < 0) {\n+\t\t\t\tif (total_written)\n+\t\t\t\t\tgoto out;\n+\t\t\t\treturn bytes_written;\n+\t\t\t}\n+\t\t\tif (!bytes_written)\n+\t\t\t\tgoto out;\n+\t\t\tiovec_written += bytes_written;\n+\t\t\ttotal_written += bytes_written;\n+\t\t}\n+\t}\n+\n+out:\n+\treturn (ssize_t) total_written;\n+}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 5feb582558..ccb3f71881 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -459,6 +459,7 @@ ifeq ($(uname_S),Windows)\n \tSANE_TOOL_PATH ?= $(msvc_bin_dir_msys)\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n+\tNO_WRITEV = YesPlease\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n \tNO_LIBGEN_H = YesPlease\n \tNO_POLL = YesPlease\n@@ -674,6 +675,7 @@ ifeq ($(uname_S),MINGW)\n \tpathsep = ;\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n+\tNO_WRITEV = YesPlease\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n \tNO_LIBGEN_H = YesPlease\n \tNO_POLL = YesPlease\ndiff --git a/meson.build b/meson.build\nindex 4b536e0124..381974ab57 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1414,6 +1414,7 @@ checkfuncs = {\n   'initgroups' : [],\n   'strtoumax' : ['strtoumax.c', 'strtoimax.c'],\n   'pread' : ['pread.c'],\n+  'writev' : ['writev.c'],\n }\n \n if host_machine.system() == 'windows'\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538860","messageId":"20260313-pks-upload-pack-write-contention-v4-6-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 06/10] wrapper: introduce writev(3p) wrappers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:17Z","receivedAt":"2026-03-13T06:45:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In the preceding commit we have added a compatibility wrapper for the\nwritev(3p) syscall. Introduce some generic wrappers for this function\nthat we nowadays take for granted in the Git codebase.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++\n wrapper.h      |  9 +++++++++\n write-or-die.c |  8 ++++++++\n write-or-die.h |  1 +\n 4 files changed, 59 insertions(+)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 16f5a63fbb..be8fa575e6 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -323,6 +323,47 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)\n \treturn total;\n }\n \n+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)\n+{\n+\tssize_t total_written = 0;\n+\n+\twhile (iovcnt) {\n+\t\tssize_t bytes_written = writev(fd, iov, iovcnt);\n+\t\tif (bytes_written < 0) {\n+\t\t\tif (errno == EINTR || errno == EAGAIN)\n+\t\t\t\tcontinue;\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (!bytes_written) {\n+\t\t\terrno = ENOSPC;\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\ttotal_written += bytes_written;\n+\n+\t\t/*\n+\t\t * We first need to discard any iovec entities that have been\n+\t\t * fully written.\n+\t\t */\n+\t\twhile (iovcnt && (size_t)bytes_written >= iov->iov_len) {\n+\t\t\tbytes_written -= iov->iov_len;\n+\t\t\tiov++;\n+\t\t\tiovcnt--;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Finally, we need to adjust the last iovec in case we have\n+\t\t * performed a partial write.\n+\t\t */\n+\t\tif (iovcnt && bytes_written) {\n+\t\t\tiov->iov_base = (char *) iov->iov_base + bytes_written;\n+\t\t\tiov->iov_len -= bytes_written;\n+\t\t}\n+\t}\n+\n+\treturn total_written;\n+}\n+\n ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)\n {\n \tchar *p = buf;\ndiff --git a/wrapper.h b/wrapper.h\nindex 15ac3bab6e..27519b32d1 100644\n--- a/wrapper.h\n+++ b/wrapper.h\n@@ -47,6 +47,15 @@ ssize_t read_in_full(int fd, void *buf, size_t count);\n ssize_t write_in_full(int fd, const void *buf, size_t count);\n ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);\n \n+/*\n+ * Try to write all iovecs. Returns -1 in case an error occurred with a proper\n+ * errno set, the number of bytes written otherwise.\n+ *\n+ * Note that the iovec will be modified as a result of this call to adjust for\n+ * partial writes!\n+ */\n+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt);\n+\n static inline ssize_t write_str_in_full(int fd, const char *str)\n {\n \treturn write_in_full(fd, str, strlen(str));\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 01a9a51fa2..5f522fb728 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -96,6 +96,14 @@ void write_or_die(int fd, const void *buf, size_t count)\n \t}\n }\n \n+void writev_or_die(int fd, struct iovec *iov, int iovlen)\n+{\n+\tif (writev_in_full(fd, iov, iovlen) < 0) {\n+\t\tcheck_pipe(errno);\n+\t\tdie_errno(\"writev error\");\n+\t}\n+}\n+\n void fwrite_or_die(FILE *f, const void *buf, size_t count)\n {\n \tif (fwrite(buf, 1, count, f) != count)\ndiff --git a/write-or-die.h b/write-or-die.h\nindex 65a5c42a47..ae3d7d88b8 100644\n--- a/write-or-die.h\n+++ b/write-or-die.h\n@@ -7,6 +7,7 @@ void fprintf_or_die(FILE *, const char *fmt, ...);\n void fwrite_or_die(FILE *f, const void *buf, size_t count);\n void fflush_or_die(FILE *f);\n void write_or_die(int fd, const void *buf, size_t count);\n+void writev_or_die(int fd, struct iovec *iov, int iovlen);\n \n /*\n  * These values are used to help identify parts of a repository to fsync.\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538859","messageId":"20260313-pks-upload-pack-write-contention-v4-7-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 07/10] sideband: use writev(3p) to send pktlines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:18Z","receivedAt":"2026-03-13T06:45:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Every pktline that we send out via `send_sideband()` currently requires\ntwo syscalls: one to write the pktline's length, and one to send its\ndata. This typically isn't all that much of a problem, but under extreme\nload the syscalls may cause contention in the kernel.\n\nRefactor the code to instead use the newly introduced writev(3p) infra\nso that we can send out the data with a single syscall. This reduces the\nnumber of syscalls from around 133,000 calls to write(3p) to around\n67,000 calls to writev(3p).\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n sideband.c | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex ea7c25211e..1ed6614eaf 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -264,6 +264,7 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma\n \tconst char *p = data;\n \n \twhile (sz) {\n+\t\tstruct iovec iov[2];\n \t\tunsigned n;\n \t\tchar hdr[5];\n \n@@ -273,12 +274,19 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma\n \t\tif (0 <= band) {\n \t\t\txsnprintf(hdr, sizeof(hdr), \"%04x\", n + 5);\n \t\t\thdr[4] = band;\n-\t\t\twrite_or_die(fd, hdr, 5);\n+\t\t\tiov[0].iov_base = hdr;\n+\t\t\tiov[0].iov_len = 5;\n \t\t} else {\n \t\t\txsnprintf(hdr, sizeof(hdr), \"%04x\", n + 4);\n-\t\t\twrite_or_die(fd, hdr, 4);\n+\t\t\tiov[0].iov_base = hdr;\n+\t\t\tiov[0].iov_len = 4;\n \t\t}\n-\t\twrite_or_die(fd, p, n);\n+\n+\t\tiov[1].iov_base = (void *) p;\n+\t\tiov[1].iov_len = n;\n+\n+\t\twritev_or_die(fd, iov, ARRAY_SIZE(iov));\n+\n \t\tp += n;\n \t\tsz -= n;\n \t}\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538861","messageId":"20260313-pks-upload-pack-write-contention-v4-8-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 08/10] csum-file: introduce `hashfd_ext()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:19Z","receivedAt":"2026-03-13T06:45:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a new `hashfd_ext()` function that takes an options structure.\nThis function will replace `hashd_throughput()` in the next commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n csum-file.c | 22 +++++++++++++---------\n csum-file.h | 14 ++++++++++++++\n 2 files changed, 27 insertions(+), 9 deletions(-)\n\ndiff --git a/csum-file.c b/csum-file.c\nindex 6e21e3cac8..a50416247e 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -161,17 +161,16 @@ struct hashfile *hashfd_check(const struct git_hash_algo *algop,\n \treturn f;\n }\n \n-static struct hashfile *hashfd_internal(const struct git_hash_algo *algop,\n-\t\t\t\t\tint fd, const char *name,\n-\t\t\t\t\tstruct progress *tp,\n-\t\t\t\t\tsize_t buffer_len)\n+struct hashfile *hashfd_ext(const struct git_hash_algo *algop,\n+\t\t\t    int fd, const char *name,\n+\t\t\t    const struct hashfd_options *opts)\n {\n \tstruct hashfile *f = xmalloc(sizeof(*f));\n \tf->fd = fd;\n \tf->check_fd = -1;\n \tf->offset = 0;\n \tf->total = 0;\n-\tf->tp = tp;\n+\tf->tp = opts->progress;\n \tf->name = name;\n \tf->do_crc = 0;\n \tf->skip_hash = 0;\n@@ -179,8 +178,8 @@ static struct hashfile *hashfd_internal(const struct git_hash_algo *algop,\n \tf->algop = unsafe_hash_algo(algop);\n \tf->algop->init_fn(&f->ctx);\n \n-\tf->buffer_len = buffer_len;\n-\tf->buffer = xmalloc(buffer_len);\n+\tf->buffer_len = opts->buffer_len ? opts->buffer_len : 128 * 1024;\n+\tf->buffer = xmalloc(f->buffer_len);\n \tf->check_buffer = NULL;\n \n \treturn f;\n@@ -194,7 +193,8 @@ struct hashfile *hashfd(const struct git_hash_algo *algop,\n \t * measure the rate of data passing through this hashfile,\n \t * use a larger buffer size to reduce fsync() calls.\n \t */\n-\treturn hashfd_internal(algop, fd, name, NULL, 128 * 1024);\n+\tstruct hashfd_options opts = { 0 };\n+\treturn hashfd_ext(algop, fd, name, &opts);\n }\n \n struct hashfile *hashfd_throughput(const struct git_hash_algo *algop,\n@@ -206,7 +206,11 @@ struct hashfile *hashfd_throughput(const struct git_hash_algo *algop,\n \t * size so the progress indicators arrive at a more\n \t * frequent rate.\n \t */\n-\treturn hashfd_internal(algop, fd, name, tp, 8 * 1024);\n+\tstruct hashfd_options opts = {\n+\t\t.progress = tp,\n+\t\t.buffer_len = 8 * 1024,\n+\t};\n+\treturn hashfd_ext(algop, fd, name, &opts);\n }\n \n void hashfile_checkpoint_init(struct hashfile *f,\ndiff --git a/csum-file.h b/csum-file.h\nindex 07ae11024a..a03b60120d 100644\n--- a/csum-file.h\n+++ b/csum-file.h\n@@ -45,6 +45,20 @@ int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);\n #define CSUM_FSYNC\t\t2\n #define CSUM_HASH_IN_STREAM\t4\n \n+struct hashfd_options {\n+\t/*\n+\t * Throughput progress that counts the number of bytes that have been\n+\t * hashed.\n+\t */\n+\tstruct progress *progress;\n+\n+\t/* The length of the buffer that shall be used read read data. */\n+\tsize_t buffer_len;\n+};\n+\n+struct hashfile *hashfd_ext(const struct git_hash_algo *algop,\n+\t\t\t    int fd, const char *name,\n+\t\t\t    const struct hashfd_options *opts);\n struct hashfile *hashfd(const struct git_hash_algo *algop,\n \t\t\tint fd, const char *name);\n struct hashfile *hashfd_check(const struct git_hash_algo *algop,\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538863","messageId":"20260313-pks-upload-pack-write-contention-v4-9-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 09/10] csum-file: drop `hashfd_throughput()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:20Z","receivedAt":"2026-03-13T06:45:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `hashfd_throughput()` function is used by a single callsite in\ngit-pack-objects(1). In contrast to `hashfd()`, this function uses a\nprogress meter to measure throughput and a smaller buffer length so that\nthe progress meter can provide more granular metrics.\n\nWe're going to change that caller in the next commit to be a bit more\nspecific to packing objects. As such, `hashfd_throughput()` will be a\nsomewhat unfitting mechanism for any potential new callers.\n\nDrop the function and replace it with a call to `hashfd_ext()`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/pack-objects.c | 19 +++++++++++++++----\n csum-file.c            | 16 ----------------\n csum-file.h            |  2 --\n 3 files changed, 15 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex c1ee4d5ed7..f5cb80e870 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1330,11 +1330,22 @@ static void write_pack_file(void)\n \t\tunsigned char hash[GIT_MAX_RAWSZ];\n \t\tchar *pack_tmp_name = NULL;\n \n-\t\tif (pack_to_stdout)\n-\t\t\tf = hashfd_throughput(the_repository->hash_algo, 1,\n-\t\t\t\t\t      \"<stdout>\", progress_state);\n-\t\telse\n+\t\tif (pack_to_stdout) {\n+\t\t\t/*\n+\t\t\t * Since we are expecting to report progress of the\n+\t\t\t * write into this hashfile, use a smaller buffer\n+\t\t\t * size so the progress indicators arrive at a more\n+\t\t\t * frequent rate.\n+\t\t\t */\n+\t\t\tstruct hashfd_options opts = {\n+\t\t\t\t.progress = progress_state,\n+\t\t\t\t.buffer_len = 8 * 1024,\n+\t\t\t};\n+\t\t\tf = hashfd_ext(the_repository->hash_algo, 1,\n+\t\t\t\t       \"<stdout>\", &opts);\n+\t\t} else {\n \t\t\tf = create_tmp_packfile(the_repository, &pack_tmp_name);\n+\t\t}\n \n \t\toffset = write_pack_header(f, nr_remaining);\n \ndiff --git a/csum-file.c b/csum-file.c\nindex a50416247e..5dfaca5543 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -197,22 +197,6 @@ struct hashfile *hashfd(const struct git_hash_algo *algop,\n \treturn hashfd_ext(algop, fd, name, &opts);\n }\n \n-struct hashfile *hashfd_throughput(const struct git_hash_algo *algop,\n-\t\t\t\t   int fd, const char *name, struct progress *tp)\n-{\n-\t/*\n-\t * Since we are expecting to report progress of the\n-\t * write into this hashfile, use a smaller buffer\n-\t * size so the progress indicators arrive at a more\n-\t * frequent rate.\n-\t */\n-\tstruct hashfd_options opts = {\n-\t\t.progress = tp,\n-\t\t.buffer_len = 8 * 1024,\n-\t};\n-\treturn hashfd_ext(algop, fd, name, &opts);\n-}\n-\n void hashfile_checkpoint_init(struct hashfile *f,\n \t\t\t      struct hashfile_checkpoint *checkpoint)\n {\ndiff --git a/csum-file.h b/csum-file.h\nindex a03b60120d..01472555c8 100644\n--- a/csum-file.h\n+++ b/csum-file.h\n@@ -63,8 +63,6 @@ struct hashfile *hashfd(const struct git_hash_algo *algop,\n \t\t\tint fd, const char *name);\n struct hashfile *hashfd_check(const struct git_hash_algo *algop,\n \t\t\t      const char *name);\n-struct hashfile *hashfd_throughput(const struct git_hash_algo *algop,\n-\t\t\t\t   int fd, const char *name, struct progress *tp);\n \n /*\n  * Free the hashfile without flushing its contents to disk. This only\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"},{"id":"538862","messageId":"20260313-pks-upload-pack-write-contention-v4-10-7a9668061f7f@pks.im","threadId":"65225","inReplyTo":"20260313-pks-upload-pack-write-contention-v4-0-7a9668061f7f@pks.im","subject":"[PATCH v4 10/10] builtin/pack-objects: reduce lock contention when writing packfile data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-13T06:45:21Z","receivedAt":"2026-03-13T06:45:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When running `git pack-objects --stdout` we feed the data through\n`hashfd_ext()` with a progress meter and a smaller-than-usual buffer\nlength of 8kB so that we can track throughput more granularly. But as\npackfiles tend to be on the larger side, this small buffer size may\ncause a ton of write(3p) syscalls.\n\nOriginally, the buffer we used in `hashfd()` was 8kB for all use cases.\nThis was changed though in 2ca245f8be (csum-file.h: increase hashfile\nbuffer size, 2021-05-18) because we noticed that the number of writes\ncan have an impact on performance. So the buffer size was increased to\n128kB, which improved performance a bit for some use cases.\n\nBut the commit didn't touch the buffer size for `hashd_throughput()`.\nThe reasoning here was that callers expect the progress indicator to\nupdate frequently, and a larger buffer size would of course reduce the\nupdate frequency especially on slow networks.\n\nWhile that is of course true, there was (and still is, even though it's\nnow a call to `hashfd_ext()`) only a single caller of this function in\ngit-pack-objects(1). This command is responsible for writing packfiles,\nand those packfiles are often on the bigger side. So arguably:\n\n  - The user won't care about increments of 8kB when packfiles tend to\n    be megabytes or even gigabytes in size.\n\n  - Reducing the number of syscalls would be even more valuable here\n    than it would be for multi-pack indices, which was the benchmark\n    done in the mentioned commit, as MIDXs are typically significantly\n    smaller than packfiles.\n\n  - Nowadays, many internet connections should be able to transfer data\n    at a rate significantly higher than 8kB per second.\n\nUpdate the buffer to instead have a size of `LARGE_PACKET_DATA_MAX - 1`,\nwhich translates to ~64kB. This limit was chosen because `git\npack-objects --stdout` is most often used when sending packfiles via\ngit-upload-pack(1), where packfile data is chunked into pktlines when\nusing the sideband. Furthermore, most internet connections should have a\nbandwidth signifcantly higher than 64kB/s, so we'd still be able to\nobserve progress updates at a rate of at least once per second.\n\nThis change significantly reduces the number of write(3p) syscalls from\n355,000 to 44,000 when packing the Linux repository. While this results\nin a small performance improvement on an otherwise-unused system, this\nimprovement is mostly negligible. More importantly though, it will\nreduce lock contention in the kernel on an extremely busy system where\nwe have many processes writing data at once.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/pack-objects.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex f5cb80e870..59876b024d 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -41,6 +41,7 @@\n #include \"promisor-remote.h\"\n #include \"pack-mtimes.h\"\n #include \"parse-options.h\"\n+#include \"pkt-line.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n #include \"path-walk.h\"\n@@ -1332,14 +1333,17 @@ static void write_pack_file(void)\n \n \t\tif (pack_to_stdout) {\n \t\t\t/*\n-\t\t\t * Since we are expecting to report progress of the\n-\t\t\t * write into this hashfile, use a smaller buffer\n-\t\t\t * size so the progress indicators arrive at a more\n-\t\t\t * frequent rate.\n+\t\t\t * This command is most often invoked via\n+\t\t\t * git-upload-pack(1), which will typically chunk data\n+\t\t\t * into pktlines. As such, we use the maximum data\n+\t\t\t * length of them as buffer length.\n+\t\t\t *\n+\t\t\t * Note that we need to subtract one though to\n+\t\t\t * accomodate for the sideband byte.\n \t\t\t */\n \t\t\tstruct hashfd_options opts = {\n \t\t\t\t.progress = progress_state,\n-\t\t\t\t.buffer_len = 8 * 1024,\n+\t\t\t\t.buffer_len = LARGE_PACKET_DATA_MAX - 1,\n \t\t\t};\n \t\t\tf = hashfd_ext(the_repository->hash_algo, 1,\n \t\t\t\t       \"<stdout>\", &opts);\n\n-- \n2.53.0.904.g2727be2e99.dirty\n\n"}]}