Subject: Re: [PATCH 1/1] Improve progress display in kB range.

From: Johannes Sixt <[email-masked]>
Date: Thu, 23 Apr 2009 08:23:06 +0200

James Cloos schrieb:
>>>>>> "Junio" == Junio C Hamano <[email-masked]> writes:
> 
> Junio> If input_buffer[ret] _were_ the last octet read, [the] loop would
> Junio> be discarding that octet when [it] call[s] more than one xread()
> Junio> to fill the buffer.
> 
> That did sem more likely, but I thought I'd throw out the possibility.
> 
> I just tried instrumenting fill().
> 
> In the first call to fill() during a fetch, xread() returns 4096.  In
> the second call to fill(), xread() returns 4095.  In all subsequent
> calls to fill(), xread() returns 4096 again.
> 
> So, after the second call to xread(), consumed_bytes & 0xFFF == 0xFFF.
> 
> It always follows the pattern:
> 
> xread() read 0x1000 from fd 0 at 0x80a0900
> ret = 0x1000
> consumed_bytes = 0
> input_len = 0x1000
> 
> xread() read 0xFFF from fd 0 at 0x80a0900
> ret = 0xFFF
> consumed_bytes = 0x1000
> input_len = 0xFFF
> 
> xread() read 0x1000 from fd 0 at 0x80a0900
> ret = 0x1000
> consumed_bytes = 0x1FFF
> input_len = 0x1000
> 
> with all subsequent calls reading 0x1000 and adding 0x1000 to
> consumed_bytes, until the final chuck of tha pack is read.
> 
> Also, all calls to xread() where the status lines are being sent from
> the remote server also return 0x1000 octets.  Only the second chunck of
> a pack ever returns 0xFFF.
> 
> I've tested against a number of remote servers and the pattern holds for
> a wide range of remote server versions.
> 
> The pattern also holds for clones over ssh.
> 
> Does anyone have an idea of why the second call to read(2), when
> receiving a pack from a remote, always leaves the last octet of the
> buffer free, whereas all other read(2)s fill it?

You could instrument upload-pack.c. There is an xread() around l.237; does
it aways return 4096? And send_client_data() at around l.254; when does it
send 4096 and when 4095 bytes? Read the comment in this if-branch.

upload-pack is run on the server-side; therefore, you can test this only
on local repositories (unless you can replace upload-pack on the server, too).

-- Hannes