public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Matt Smiley <msmiley@gitlab.com>
Subject: Re: [PATCH 2/2] upload-pack: reduce lock contention when writing packfile data
Date: Fri, 27 Feb 2026 13:04:05 +0000	[thread overview]
Message-ID: <aaGWRWcxEtLD1OlK@fruit.crustytoothpaste.net> (raw)
In-Reply-To: <20260227-pks-upload-pack-write-contention-v1-2-7166fe255704@pks.im>

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

On 2026-02-27 at 11:23:01, Patrick Steinhardt wrote:
> 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];

This seems mostly reasonable and well-explained.  The one question I
have is this: how does this work when packfile generation is actually
very slow (or when the connection is slow) and we need to send data
every so often to keep the connection alive?

I just want to make sure we're not breaking the keepalive sideband case
when that's necessary, but of course I have no objections to improving
performance and reducing overhead.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

  reply	other threads:[~2026-02-27 13:09 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-27 11:22 [PATCH 0/2] upload-pack: reduce lock contention when writing packfile data Patrick Steinhardt
2026-02-27 11:23 ` [PATCH 1/2] upload-pack: fix debug statement when flushing " Patrick Steinhardt
2026-02-27 11:23 ` [PATCH 2/2] upload-pack: reduce lock contention when writing " Patrick Steinhardt
2026-02-27 13:04   ` brian m. carlson [this message]
2026-02-27 18:14     ` Patrick Steinhardt
2026-02-27 17:29   ` Junio C Hamano
2026-02-27 19:37   ` Jeff King
2026-03-02 12:12     ` Patrick Steinhardt
2026-03-02 18:20       ` Jeff King
2026-03-03  9:31         ` Patrick Steinhardt
2026-03-03 13:35           ` Jeff King
2026-03-03 13:47             ` Patrick Steinhardt
2026-03-03 15:00 ` [PATCH v2 00/10] " Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 01/10] upload-pack: fix debug statement when flushing " Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 02/10] upload-pack: adapt keepalives based on buffering Patrick Steinhardt
2026-03-05  0:56     ` Jeff King
2026-03-10 12:08       ` Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 03/10] upload-pack: reduce lock contention when writing packfile data Patrick Steinhardt
2026-03-05  1:16     ` Jeff King
2026-03-10 12:14       ` Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 04/10] git-compat-util: introduce `cast_size_t_to_ssize_t()` Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 05/10] compat/posix: introduce writev(3p) wrapper Patrick Steinhardt
2026-03-04 22:01     ` Junio C Hamano
2026-03-05  0:37       ` Jeff King
2026-03-05  2:16         ` brian m. carlson
2026-03-05  6:39           ` Johannes Sixt
2026-03-05 22:22             ` brian m. carlson
2026-03-10 12:09               ` Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 06/10] wrapper: introduce writev(3p) wrappers Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 07/10] sideband: use writev(3p) to send pktlines Patrick Steinhardt
2026-03-04 22:05     ` Junio C Hamano
2026-03-03 15:00   ` [PATCH v2 08/10] csum-file: introduce `hashfd_ext()` Patrick Steinhardt
2026-03-04 22:11     ` Junio C Hamano
2026-03-10 12:09       ` Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 09/10] csum-file: drop `hashfd_throughput()` Patrick Steinhardt
2026-03-03 15:00   ` [PATCH v2 10/10] builtin/pack-objects: reduce lock contention when writing packfile data Patrick Steinhardt
2026-03-10 13:24 ` [PATCH v3 00/10] upload-pack: " Patrick Steinhardt
2026-03-10 13:24   ` [PATCH v3 01/10] upload-pack: fix debug statement when flushing " Patrick Steinhardt
2026-03-10 13:24   ` [PATCH v3 02/10] upload-pack: adapt keepalives based on buffering Patrick Steinhardt
2026-03-10 13:24   ` [PATCH v3 03/10] upload-pack: prefer flushing data over sending keepalive Patrick Steinhardt
2026-03-10 17:09     ` Junio C Hamano
2026-03-10 17:43       ` Patrick Steinhardt
2026-03-10 13:25   ` [PATCH v3 04/10] upload-pack: reduce lock contention when writing packfile data Patrick Steinhardt
2026-03-10 13:25   ` [PATCH v3 05/10] compat/posix: introduce writev(3p) wrapper Patrick Steinhardt
2026-03-10 16:59     ` Junio C Hamano
2026-03-10 13:25   ` [PATCH v3 06/10] wrapper: introduce writev(3p) wrappers Patrick Steinhardt
2026-03-10 13:25   ` [PATCH v3 07/10] sideband: use writev(3p) to send pktlines Patrick Steinhardt
2026-03-10 13:25   ` [PATCH v3 08/10] csum-file: introduce `hashfd_ext()` Patrick Steinhardt
2026-03-10 13:25   ` [PATCH v3 09/10] csum-file: drop `hashfd_throughput()` Patrick Steinhardt
2026-03-10 13:25   ` [PATCH v3 10/10] builtin/pack-objects: reduce lock contention when writing packfile data Patrick Steinhardt
2026-03-10 17:11   ` [PATCH v3 00/10] upload-pack: " Junio C Hamano
2026-03-10 20:56   ` Johannes Sixt
2026-03-11  6:27     ` Patrick Steinhardt
2026-03-13  6:45 ` [PATCH v4 " Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 01/10] upload-pack: fix debug statement when flushing " Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 02/10] upload-pack: adapt keepalives based on buffering Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 03/10] upload-pack: prefer flushing data over sending keepalive Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 04/10] upload-pack: reduce lock contention when writing packfile data Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 05/10] compat/posix: introduce writev(3p) wrapper Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 06/10] wrapper: introduce writev(3p) wrappers Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 07/10] sideband: use writev(3p) to send pktlines Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 08/10] csum-file: introduce `hashfd_ext()` Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 09/10] csum-file: drop `hashfd_throughput()` Patrick Steinhardt
2026-03-13  6:45   ` [PATCH v4 10/10] builtin/pack-objects: reduce lock contention when writing packfile data Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aaGWRWcxEtLD1OlK@fruit.crustytoothpaste.net \
    --to=sandals@crustytoothpaste.net \
    --cc=git@vger.kernel.org \
    --cc=msmiley@gitlab.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox