From: Junio C Hamano <gitster@pobox.com>
To: "Shawn O. Pearce" <spearce@spearce.org>
Cc: Robin Rosenberg <robin.rosenberg@dewire.com>, git@vger.kernel.org
Subject: Re: [JGIT PATCH 2/2] Decrease the fetch pack client buffer to the lower minimum
Date: Sun, 10 May 2009 17:43:20 -0700 [thread overview]
Message-ID: <7vfxfctqon.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1241995685-13260-2-git-send-email-spearce@spearce.org> (Shawn O. Pearce's message of "Sun\, 10 May 2009 15\:48\:05 -0700")
"Shawn O. Pearce" <spearce@spearce.org> writes:
> This is the lowest buffer size we actually require to keep the
> client and server sides from deadlocking against each other.
Is this about the fetch-pack protocol where
(1) upload-pack shows what it has; fetch-pack keeps reading until it sees
a flush; then
(2) fetch-pack shows what it wants; upload-pack keeps reading; then
(3) fetch-pack sends a bunch of have's, followed by a flush; upload-pack
keeps reading and then responds with an ACK-continue or NAK, which
fetch-pack reads; this step continues zero or more times; and then
finally
(4) fetch-pack sends a bunch of have's, followed by a flush; upload-pack
keeps reading and then responds with an ACK, fetch-pack says done.
Where do you need "enough buffer"? The conversation looks very much "it's
my turn to talk", "now it's your turn to talk and I'll wait until I hear
from you", to me. I am puzzled.
> Also added documentation, and renamed the symbol to better match
> its real purpose; naming the lower threshold we can allow for a
> buffer.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> .../jgit/transport/BasePackFetchConnection.java | 10 +++++++++-
> .../org/spearce/jgit/transport/TransportLocal.java | 2 +-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
> index eaa94bd..1d1b801 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
> @@ -89,7 +89,15 @@
> */
> private static final int MAX_HAVES = 256;
>
> - protected static final int MAX_CLIENT_BUFFER = MAX_HAVES * 46 + 1024;
> + /**
> + * Amount of data the client sends before starting to read.
> + * <p>
> + * Any output stream given to the client must be able to buffer this many
> + * bytes before the client will stop writing and start reading from the
> + * input stream. If the output stream blocks before this many bytes are in
> + * the send queue, the system will deadlock.
> + */
> + protected static final int MIN_CLIENT_BUFFER = 2 * 32 * 46 + 4;
>
> static final String OPTION_INCLUDE_TAG = "include-tag";
>
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
> index cffdba1..428f73e 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
> @@ -175,7 +175,7 @@ InternalLocalFetchConnection() throws TransportException {
> // force the buffer to be big enough, otherwise it
> // will deadlock both threads.
> {
> - buffer = new byte[MAX_CLIENT_BUFFER];
> + buffer = new byte[MIN_CLIENT_BUFFER];
> }
> };
> out_w = new PipedOutputStream(out_r);
> --
> 1.6.3.48.g99c76
next prev parent reply other threads:[~2009-05-11 0:46 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-10 22:48 [JGIT PATCH 1/2] Fix deadlock in native git protocol client for upload-pack Shawn O. Pearce
2009-05-10 22:48 ` [JGIT PATCH 2/2] Decrease the fetch pack client buffer to the lower minimum Shawn O. Pearce
2009-05-11 0:43 ` Junio C Hamano [this message]
2009-05-11 0:55 ` Shawn O. Pearce
2009-05-11 3:51 ` Junio C Hamano
2009-05-11 14:10 ` Shawn O. Pearce
2009-05-11 14:23 ` (unknown), Carl Mercier
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=7vfxfctqon.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
--cc=spearce@spearce.org \
/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;
as well as URLs for NNTP newsgroup(s).