From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 27 Feb 2026 17:29:00 GMT
Subject: Re: [PATCH 2/2] upload-pack: reduce lock contention when writing packfile data
Message-ID: <xmqqpl5qrt7n.fsf@gitster.g>
In-Reply-To: <20260227-pks-upload-pack-write-contention-v1-2-7166fe255704@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> ...
> write(3p) call. Even worse, when the sideband is enabled we end up
> matching one read with _two_ writes: one for the pkt-line length, and
> one for the packfile data.
>
> Extend our use of the buffering infrastructure so that we soak up bytes
> until the buffer is filled up at least 2/3rds of its capacity. The
> change is relatively simple to implement as we already know to flush the
> buffer in `create_pack_file()` after git-pack-objects(1) has finished.
>
> This significantly reduces the number of write(3p) syscalls we need to
> do. Before this change, cloning the Linux repository resulted in around
> 400,000 write(3p) syscalls. With the buffering in place we only do
> around 130,000 syscalls.
>
> Now we could of course go even further and make sure that we always fill
> up the whole buffer. But this might cause an increase in read(3p)
> syscalls, and some tests show that this only reduces the number of
> write(3p) syscalls from 130,000 to 100,000. So overall this doesn't seem
> worth it.

Very well reasoned.  I love this size ratio between explanation and
code change ;-)

>
> Helped-by: Matt Smiley <msmiley@gitlab.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  upload-pack.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/upload-pack.c b/upload-pack.c
> index c2643c0295..f8ba245616 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -270,6 +270,13 @@ static int relay_pack_data(int pack_objects_out, struct output_state *os,
>  		}
>  	}
>  
> +	/*
> +	 * Make sure that we buffer some data before sending it to the client.
> +	 * This significantly reduces the number of write(3p) syscalls.
> +	 */
> +	if (readsz && os->used < (LARGE_PACKET_DATA_MAX * 2 / 3))
> +		return readsz;
> +
>  	if (os->used > 1) {
>  		send_client_data(1, os->buffer, os->used - 1, use_sideband);
>  		os->buffer[0] = os->buffer[os->used - 1];