In a subsequent commit we're going to add the first caller to writev(3p). Introduce a compatibility wrapper for this syscall that we can use on systems that don't have this syscall.

The syscall exists on modern Unixes like Linux and macOS, and seemingly even for NonStop according to [1]. It doesn't seem to exist on Windows though.

[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf [2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                            |  4 ++++
 compat/posix.h                      | 14 ++++++++++++
 compat/writev.c                     | 44 +++++++++++++++++++++++++++++++++++++
 config.mak.uname                    |  2 ++
 contrib/buildsystems/CMakeLists.txt |  6 ++++-
 meson.build                         |  1 +
 6 files changed, 70 insertions(+), 1 deletion(-)

Hi,

this patch series reintroduces the writev(3p) wrapper. This wrapper was originally introduced as part of Git 2.54 [1], but was ejected due to issues on NonStop [2].

This patch series here revives the effort with a couple of fixes on top:

  - It picks Dscho's fix for CMake [3].

  - It picks a fix for NonStop [4] and polishes it a bit.

  - It adapts one more site to demonstrate that its usefulness is not
    limited to a single callsite, only.

Furthermore, I have included benchmarks now that demonstrate the benefits to make this series a bit more appealing. Ultimately, I'd be fine if we say we rather don't want to go this way though. I merely wanted to tie some loose ends that I left dangling.

That, and it's nice to not work on pluggable object databases once in a while.

Thanks!

Patrick

[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im> [2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com> [3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com> [4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>

---
Patrick Steinhardt (5):
      compat/posix: introduce writev(3p) wrapper
      wrapper: introduce writev(3p) wrappers
      wrapper: properly handle MAX_IO_SIZE in writev(3p)
      sideband: use writev(3p) to send pktlines
      fast-import: use writev(3p) to send cat-blob responses

 Makefile                            |  4 ++
 builtin/fast-import.c               | 18 +++++++--
 compat/posix.h                      | 14 +++++++
 compat/writev.c                     | 44 +++++++++++++++++++++
 config.mak.uname                    |  2 +
 contrib/buildsystems/CMakeLists.txt |  6 ++-
 meson.build                         |  1 +
 sideband.c                          | 14 +++++--
 wrapper.c                           | 78 +++++++++++++++++++++++++++++++++++++
 wrapper.h                           | 10 +++++
 write-or-die.c                      |  8 ++++
 write-or-die.h                      |  1 +
 12 files changed, 193 insertions(+), 7 deletions(-)

--- base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9 change-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9

In the preceding commit we have added a compatibility wrapper for the writev(3p) syscall. Introduce some generic wrappers for this function that we nowadays take for granted in the Git codebase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 wrapper.h      |  9 +++++++++
 write-or-die.c |  8 ++++++++
 write-or-die.h |  1 +
 4 files changed, 59 insertions(+)

Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which limits the maximum number of bytes we're allowed to write in a single call. We already handle this limit properly in `xwrite()`, but we have recently introduced wrappers for writev(3p) where we don't. This will cause the syscall to return EINVAL in case somebody passes an iovec entry to writev(3p) that is larger than `MAX_IO_SIZE`.

Introduce a new function `xwritev()` that is similar to `xwrite()` in that it handles such platform-specific nuances:

  - We only pass the leading iovec entries to writev(3p) that fit into
    `MAX_IO_SIZE`, pretending that the underlying syscall performed a
    short write. This mirrors how `xwrite()` chomps overly large
    requests before handing them to write(3p). As a consequence, callers
    will never see writev(3p)'s EINVAL error for requests whose summed
    length would overflow an ssize_t, but observe a short write instead.

  - If already the first iovec entry exceeds the limit we instead punt
    to `xwrite()`, which knows to handle this case for us.

  - We restart the underlying syscall on EINTR and EAGAIN, just like
    `xwrite()` does for write(3p).

Adapt `writev_in_full()` to use this new wrapper. With the retry logic now living in `xwritev()`, the calling loop becomes the exact mirror image of `write_in_full()`, which also retains the responsibility of translating a zero-length write into ENOSPC.

Reported-by: Randall Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 wrapper.h |  1 +
 2 files changed, 43 insertions(+), 5 deletions(-)

Every pktline that we send out via `send_sideband()` currently requires two syscalls: one to write the pktline's length, and one to send its data. This typically isn't all that much of a problem, but under extreme load the syscalls may cause contention in the kernel.

Refactor the code to instead use the newly introduced writev(3p) infra so that we can send out the data with a single syscall. This reduces the number of syscalls from around 133,000 calls to write(3p) to around 67,000 calls to writev(3p).

This change leads to a performance improvement for git-upload-pack(1), but we have to cheat a bit to really make it measurable. Usually, the time is strongly dominated by generating the packfile itself. But if we precompute the pack and serve it via the pack-objects hook then we can essentially eliminate that overhead. The following setup is executed in the Git repository:

  $ cat >request <<-EOF
  0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress
  00000009done
  EOF
  $ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack
  $ cat >hook <<-EOF
  #!/bin/sh
  cat >/dev/null
  cat "$(pwd)"/pack
  EOF
  $ chmod u+x hook
  $ git -c uploadpack.packObjectsHook="$(pwd)"/hook upload-pack . <request

Benchmarking the last command leads to the following results:

  Benchmark 1: HEAD~
    Time (mean ± σ):     192.9 ms ±   0.6 ms    [User: 106.5 ms, System: 95.3 ms]
    Range (min … max):   191.7 ms … 194.1 ms    50 runs

  Benchmark 2: HEAD
    Time (mean ± σ):     141.1 ms ±   0.7 ms    [User: 63.2 ms, System: 86.6 ms]
    Range (min … max):   139.8 ms … 142.7 ms    50 runs

  Summary
    HEAD ran
      1.37 ± 0.01 times faster than HEAD~

This might not be impressive in absolute numbers when you also take into account the time it takes to generate the packfile itself. But GitLab (and supposedly other forges) have caching mechanisms in place that work exactly like the above setup, where repeated incoming requests can be served from the same cached packfile. And in those cases, the impact is sizeable.

More importantly though, as hinted at above, GitLab has observed in the past that with enough cache hits we eventually start to saturate a semaphore in the Linux kernel itself in the pipe write path. This bottleneck is being moved a bit by having to do less syscalls.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 sideband.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

When answering a `cat-blob` command, `cat_blob()` issues three separate calls to write(3p) on the cat-blob fd: one for the header line, one for the full blob payload, and one for the trailing newline. Frontends like git-filter-repo issue these commands in bulk, once per rewritten blob, so the syscall overhead adds up.

Use `writev_in_full()` to send all three parts with a single syscall.

This can be benchmarked with the following setup:

    $ git cat-file --unordered --filter=object:type=blob
        --batch-check='cat-blob %(objectname)' --batch-all-objects >request
    $ git fast-import --cat-blob-fd=3 <request

Executing this with 100,000 objects in linux.git:

  Benchmark 1: HEAD~
    Time (mean ± σ):      1.320 s ±  0.003 s    [User: 1.154 s, System: 0.161 s]
    Range (min … max):    1.314 s …  1.324 s    10 runs

  Benchmark 2: HEAD
    Time (mean ± σ):      1.270 s ±  0.022 s    [User: 1.133 s, System: 0.132 s]
    Range (min … max):    1.209 s …  1.282 s    10 runs

  Summary
    HEAD ran
      1.04 ± 0.02 times faster than HEAD~

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fast-import.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Hi,

That feels like it could overflow.

    Simon

Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:

Please don't call the function "writev" so that nobody associates it with the guarantees that only POSIX provides, but none of the emulations. Call it "write_gather", for example.

Also, clearly document that its only purpose is to reduce sequences of write() calls to a single function call, but that the additional writev guarantees are not needed.

A range-diff to the earlier round would have been very helpful.

-- Hannes

Simon Richter <Simon.Richter@hogyros.de> writes:

Isn't it checking if it would overflow (and dying if so)?

Ah, wait. The addition "(iov[i].iov_len + sum)" can indeed wrap around, and comparing it with the maximum value of ssize_t wouldn't catch that. Is that what you mean?

Would something like this:

    if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||
	iov[i].iov_len + sum < iov[i].iov_len ||
	maximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)

work better to catch the three cases independently?

 (1) The value is already too large on its own.
 (2) Adding them together would cause an unsigned wrap-around.
 (3) The sum does not wrap around, but it exceeds the maximum
     representable value of ssize_t anyway.

Junio C Hamano <gitster@pobox.com> writes:

Actually, looking at it again, I think the original code is safe after all, because:

 * "sum", even though it is a size_t, is checked inside the loop to
   ensure it stays below the maximum value of ssize_t each time it
   gets a new value.
 * iov[i].iov_len is checked to ensure it does not exceed the
   maximum value of ssize_t by the first part of the condition.

If both values are less than or equal to the maximum value of ssize_t, their sum is at most twice that limit. For an N-bit size_t, this sum is at most (2^N - 2), which can be computed safely without any unsigned wrap-around.

So...?

Johannes Sixt <j6t@kdbg.org> writes:

It is philosophically more "pure" to have a two-level abstraction where write_gather(), which may be inspired by writev(2) but with specific subset of semantics that the application needs, is used by the application and have platforms with good enough writev(2) to implement it in terms of it. Other platforms may implement it differently, like a series of write(2) calls, and as long as it fulfills the need of write_gather(), we are OK.

Doing so would also help in a minuscule way to avoid adding to the complaints we sometimes hear that our internal implementation assumes platform support for POSIX API and semantics way too much even when we do not need to.

So I do not mind going in that direction. It feels a slightly roundabout approach, but in the longer run, I think it would place us in a much better place.

I think Patrick's writev(2) follows the pattern our previous compat/ routines have taken. We use real writev(2) where it is available, and in the fake implementations in compat/ we have comments that essentially say "the real function offers X, Y, and Z, but we only want X and Z and do not need Y, so this implementation does not support Y". It is harder to maintain because the application side may be tempted over time to start depending on Y. If some platforms cannot easily provide an equivalent of the real function, it is easier for them if the rules explicitly state from the beginning that we do not require and will never require Y, needing only X and Z from either the fake or real implementation.

At that point, we are not describing the real function anymore, so your proposal to give it a specific name is one step away from that, and that step is in the right direction.

Thanks.

PS. I was going over the list of "waiting for response" topics, and this was one of them. I suspect Patrick and the GitLab team are still away at an offsite [*], so this is in no way poking him for an immediate reroll, but rather a note sent while my attention is on these stalled topics.

https://lore.kernel.org/git/amLgMqkqxR8mKIbT@pks.im/

On Thu, Jul 16, 2026 at 01:44:18PM -0700, Junio C Hamano wrote:

Yeah, I think your analysis is correct. It's quite subtle though, so maybe we should make this a bit more explicit? Something like the following patch for example:

On Mon, Jul 27, 2026 at 08:44:39AM -0700, Junio C Hamano wrote:

Yeah, I was mostly trying to follow the precedent that we currently have in our code base, where we assume POSIX functions and paper over any gaps that a specific platform has via compatibility wrappers. And I think that the compatibility wrapper we have for writev(3p) is close enough to the original semantics of it to not really matter much in practice.

I overall don't disagree that it would've been nice at times to have a higher-level interface that abstracts over such platform specifics without assuming POSIX semantics. But I'm not really sure what it buys us to rename this to `write_gather()` without rethinking the bigger approach we have to I/O. That is, what does it buy us to now diverge from the current practice, and where do we want to end up?

I feel like that's a much bigger discussion to be had, and I'm not a 100% sure whether I want to open that can of worms now. If the only thing that I need to change is to rename from writev to write_gather then I'm happy to do that. But as said, I don't really think this buys us much without the bigger discussion, so I'm a bit hesitant to do this.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

As I am not the party who needs to implement a good enough emulation, and what is hard to do in non POSIX environment that is needed for writev(2) emulation, I think that is a question for j6t.

A different way to put the same question is "what is Y in the context of the intended uses of writev(2) in our codebase"?

Thanks.

Am 05.08.26 um 18:36 schrieb Junio C Hamano:

The Y that I am thinking of primarily is the atomicity guarantee:

(See `man 2 writev`; this isn't spelled out explicitly in the Open Group Base Specification.)

This is basically unimplementable by any emulation that has to call write() multiple times.

-- Hannes

Johannes Sixt <j6t@kdbg.org> writes:

Looking at hits from 'git grep -e writev seen', the only two places we use writev() or write_gather() are:

 - fast-import, where we write out concatenation of the object
   header, the payload, and the trailing newline in three separate
   buffers in one go; and

 - sideband, where we send the length and band designator in one
   buffer and the payload in another in one go.

Neither use would work at all if we had competing writers working in parallel to write to the same pipe, regardless of whether atomicity is guaranteed.

I think it is OK to explicitly document that any writev(2) emulation is allowed to be non-atomic, and it is also OK to declare that using writev(2) in this application to allow competing writes to the same destination is a bug.

Am 05.08.26 um 20:40 schrieb Junio C Hamano:

These are fine.

But I'm not worried about current uses of writev, I'm worried about future uses: "Look, we already use writev elsewhere. Let's use it here, too, where we can take adavantage of the atomicity of the write." It's too easy to miss a note about non-atomic emulations when the function name advertises more than can be guaranteed. For this reason, I strongly suggest to use a different name.

-- Hannes

Johannes Sixt <j6t@kdbg.org> writes:

That is why I added the "it is also OK to declare" in the above.

On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:

We could of course trivially restore the non-interleaving property by only ever writing the first iovec. POSIX doesn't guarantee that the full iovec is being written, and write(3p) is already non-interleaving. It wouldn't even be less efficient compared to the current implementation, as we have to loop around write(3p) anyway in our compatibility wrapper.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

OK, by castrating the writev(2) emulation implementation to write out only the first iovec[], we are making the emulation "atomic", so there is no need to say "your emulation does not have to be atomic" and we can rely on being able to pretend that we have writev(2) available everywhere. Also, it is a bug on the programmers' side to assume that their writev() calls will not result in a short write, so it does not have to be spelled out, either, which automatically means you'd better be calling writev_in_full() and not writev() itself.

I can buy that. Clever. It means we'd need an update for [PATCH 1/5] 1ed0bc4e3b (compat/posix: introduce writev(3p) wrapper, 2026-07-16), right? The update would be a simplification that loses a lot of code (and overflow check), which is even nicer ;-).

Hi,

this patch series reintroduces the writev(3p) wrapper. This wrapper was originally introduced as part of Git 2.54 [1], but was ejected due to issues on NonStop [2].

This patch series here revives the effort with a couple of fixes on top:

  - It picks Dscho's fix for CMake [3].

  - It picks a fix for NonStop [4] and polishes it a bit.

  - It adapts one more site to demonstrate that its usefulness is not
    limited to a single callsite, only.

Furthermore, I have included benchmarks now that demonstrate the benefits to make this series a bit more appealing. Ultimately, I'd be fine if we say we rather don't want to go this way though. I merely wanted to tie some loose ends that I left dangling.

That, and it's nice to not work on pluggable object databases once in a while.

Changes in v2:
  - Adapt the writev compatibility shim to not do torn writes anymore
    across multiple iovecs. Instead, we now only write the first iovec,
    which should be fine as callers are expected to loop around writev
    anyway.
  - Link to v1: https://patch.msgid.link/20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im

Thanks!

Patrick

[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im> [2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com> [3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com> [4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>

---
Patrick Steinhardt (5):
      compat/posix: introduce writev(3p) wrapper
      wrapper: introduce writev(3p) wrappers
      wrapper: properly handle MAX_IO_SIZE in writev(3p)
      sideband: use writev(3p) to send pktlines
      fast-import: use writev(3p) to send cat-blob responses

 Makefile                            |  4 ++
 builtin/fast-import.c               | 18 +++++++--
 compat/posix.h                      | 14 +++++++
 compat/writev.c                     | 41 +++++++++++++++++++
 config.mak.uname                    |  2 +
 contrib/buildsystems/CMakeLists.txt |  6 ++-
 meson.build                         |  1 +
 sideband.c                          | 14 +++++--
 wrapper.c                           | 78 +++++++++++++++++++++++++++++++++++++
 wrapper.h                           | 10 +++++
 write-or-die.c                      |  8 ++++
 write-or-die.h                      |  1 +
 12 files changed, 190 insertions(+), 7 deletions(-)

Range-diff versus v1:

1:  69b8be6ec5 ! 1:  f519260452 compat/posix: introduce writev(3p) wrapper
    @@ compat/writev.c (new)
     +
     +ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)
     +{
    -+	size_t total_written = 0;
     +	size_t sum = 0;
     +
    ++	if (iovcnt <= 0) {
    ++		errno = EINVAL;
    ++		return -1;
    ++	}
    ++
     +	/*
     +	 * According to writev(3p), the syscall shall error with EINVAL in case
     +	 * the sum of `iov_len` overflows `ssize_t`.
     +	 */
     +	for (int i = 0; i < iovcnt; i++) {
     +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
    ++		    unsigned_add_overflows(iov[i].iov_len, sum) ||
     +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
     +			errno = EINVAL;
     +			return -1;
    @@ compat/writev.c (new)
     +		sum += iov[i].iov_len;
     +	}
     +
    ++	/*
    ++	 * We only ever write the first non-empty vector so that we can
    ++	 * guarantee the call to be non-interleaving as guaranteed by POSIX.
    ++	 * This works just fine as callers have to loop around writev anyway.
    ++	 */
     +	for (int i = 0; i < iovcnt; i++) {
    -+		const char *bytes = iov[i].iov_base;
    -+		size_t iovec_written = 0;
    -+
    -+		while (iovec_written < iov[i].iov_len) {
    -+			ssize_t bytes_written = xwrite(fd, bytes + iovec_written,
    -+						       iov[i].iov_len - iovec_written);
    -+			if (bytes_written < 0) {
    -+				if (total_written)
    -+					goto out;
    -+				return bytes_written;
    -+			}
    -+			if (!bytes_written)
    -+				goto out;
    -+			iovec_written += bytes_written;
    -+			total_written += bytes_written;
    -+		}
    ++		if (!iov[i].iov_len)
    ++			continue;
    ++		return xwrite(fd, iov[i].iov_base, iov[i].iov_len);
     +	}
     +
    -+out:
    -+	return (ssize_t) total_written;
    ++	/* When all iovec members were zero we ought to return 0 according to POSIX. */
    ++	return 0;
     +}
     
      ## config.mak.uname ##
2:  f6013a18ba = 2:  41814b6668 wrapper: introduce writev(3p) wrappers
3:  3dc0eff00b = 3:  af2e351491 wrapper: properly handle MAX_IO_SIZE in writev(3p)
4:  95c872432f = 4:  d3ba9d73d2 sideband: use writev(3p) to send pktlines
5:  4c1efb5284 = 5:  148a2c8928 fast-import: use writev(3p) to send cat-blob responses

--- base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9 change-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9

In a subsequent commit we're going to add the first caller to writev(3p). Introduce a compatibility wrapper for this syscall that we can use on systems that don't have this syscall.

The syscall exists on modern Unixes like Linux and macOS, and seemingly even for NonStop according to [1]. It doesn't seem to exist on Windows though.

[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf [2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                            |  4 ++++
 compat/posix.h                      | 14 +++++++++++++
 compat/writev.c                     | 41 +++++++++++++++++++++++++++++++++++++
 config.mak.uname                    |  2 ++
 contrib/buildsystems/CMakeLists.txt |  6 +++++-
 meson.build                         |  1 +
 6 files changed, 67 insertions(+), 1 deletion(-)

In the preceding commit we have added a compatibility wrapper for the writev(3p) syscall. Introduce some generic wrappers for this function that we nowadays take for granted in the Git codebase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 wrapper.h      |  9 +++++++++
 write-or-die.c |  8 ++++++++
 write-or-die.h |  1 +
 4 files changed, 59 insertions(+)

Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which limits the maximum number of bytes we're allowed to write in a single call. We already handle this limit properly in `xwrite()`, but we have recently introduced wrappers for writev(3p) where we don't. This will cause the syscall to return EINVAL in case somebody passes an iovec entry to writev(3p) that is larger than `MAX_IO_SIZE`.

Introduce a new function `xwritev()` that is similar to `xwrite()` in that it handles such platform-specific nuances:

  - We only pass the leading iovec entries to writev(3p) that fit into
    `MAX_IO_SIZE`, pretending that the underlying syscall performed a
    short write. This mirrors how `xwrite()` chomps overly large
    requests before handing them to write(3p). As a consequence, callers
    will never see writev(3p)'s EINVAL error for requests whose summed
    length would overflow an ssize_t, but observe a short write instead.

  - If already the first iovec entry exceeds the limit we instead punt
    to `xwrite()`, which knows to handle this case for us.

  - We restart the underlying syscall on EINTR and EAGAIN, just like
    `xwrite()` does for write(3p).

Adapt `writev_in_full()` to use this new wrapper. With the retry logic now living in `xwritev()`, the calling loop becomes the exact mirror image of `write_in_full()`, which also retains the responsibility of translating a zero-length write into ENOSPC.

Reported-by: Randall Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 wrapper.h |  1 +
 2 files changed, 43 insertions(+), 5 deletions(-)

Every pktline that we send out via `send_sideband()` currently requires two syscalls: one to write the pktline's length, and one to send its data. This typically isn't all that much of a problem, but under extreme load the syscalls may cause contention in the kernel.

Refactor the code to instead use the newly introduced writev(3p) infra so that we can send out the data with a single syscall. This reduces the number of syscalls from around 133,000 calls to write(3p) to around 67,000 calls to writev(3p).

This change leads to a performance improvement for git-upload-pack(1), but we have to cheat a bit to really make it measurable. Usually, the time is strongly dominated by generating the packfile itself. But if we precompute the pack and serve it via the pack-objects hook then we can essentially eliminate that overhead. The following setup is executed in the Git repository:

  $ cat >request <<-EOF
  0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress
  00000009done
  EOF
  $ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack
  $ cat >hook <<-EOF
  #!/bin/sh
  cat >/dev/null
  cat "$(pwd)"/pack
  EOF
  $ chmod u+x hook
  $ git -c uploadpack.packObjectsHook="$(pwd)"/hook upload-pack . <request

Benchmarking the last command leads to the following results:

  Benchmark 1: HEAD~
    Time (mean ± σ):     192.9 ms ±   0.6 ms    [User: 106.5 ms, System: 95.3 ms]
    Range (min … max):   191.7 ms … 194.1 ms    50 runs

  Benchmark 2: HEAD
    Time (mean ± σ):     141.1 ms ±   0.7 ms    [User: 63.2 ms, System: 86.6 ms]
    Range (min … max):   139.8 ms … 142.7 ms    50 runs

  Summary
    HEAD ran
      1.37 ± 0.01 times faster than HEAD~

This might not be impressive in absolute numbers when you also take into account the time it takes to generate the packfile itself. But GitLab (and supposedly other forges) have caching mechanisms in place that work exactly like the above setup, where repeated incoming requests can be served from the same cached packfile. And in those cases, the impact is sizeable.

More importantly though, as hinted at above, GitLab has observed in the past that with enough cache hits we eventually start to saturate a semaphore in the Linux kernel itself in the pipe write path. This bottleneck is being moved a bit by having to do less syscalls.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 sideband.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

When answering a `cat-blob` command, `cat_blob()` issues three separate calls to write(3p) on the cat-blob fd: one for the header line, one for the full blob payload, and one for the trailing newline. Frontends like git-filter-repo issue these commands in bulk, once per rewritten blob, so the syscall overhead adds up.

Use `writev_in_full()` to send all three parts with a single syscall.

This can be benchmarked with the following setup:

    $ git cat-file --unordered --filter=object:type=blob
        --batch-check='cat-blob %(objectname)' --batch-all-objects >request
    $ git fast-import --cat-blob-fd=3 <request

Executing this with 100,000 objects in linux.git:

  Benchmark 1: HEAD~
    Time (mean ± σ):      1.320 s ±  0.003 s    [User: 1.154 s, System: 0.161 s]
    Range (min … max):    1.314 s …  1.324 s    10 runs

  Benchmark 2: HEAD
    Time (mean ± σ):      1.270 s ±  0.022 s    [User: 1.133 s, System: 0.132 s]
    Range (min … max):    1.209 s …  1.282 s    10 runs

  Summary
    HEAD ran
      1.04 ± 0.02 times faster than HEAD~

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fast-import.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

On Thu, Aug 06, 2026 at 01:26:21PM -0700, Junio C Hamano wrote:

Yeah, exactly. The overflow check I think we should keep though to be closer to the POSIX requirements.

Patrick