Re: [JGIT PATCH 2/2] Decrease the fetch pack client buffer to the lower minimum

From: Junio C Hamano <gitster@pobox.com>
Date: May 11, 2009, 00:43 UTC
Message-ID: <7vfxfctqon.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1241995685-13260-2-git-send-email-spearce@spearce.org>

"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.

Show 46 quoted lines

> 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