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

From: James Cloos <[email-masked]>
Date: Tue, 21 Apr 2009 16:16:53 -0400

>>>>> "Nicolas" == Nicolas Pitre <[email-masked]> writes:

Nicolas> Empirical evidence on my side shows the opposite.  I just did a fetch in 
Nicolas> my kernel repo and got:

Nicolas>    Receiving objects: 100% (1373/1373), 223.36 KiB, done.

OK.  That does show that my proposed patch is incomplete.

The contrary example is from the final output, if the received pack
is less than a Meg.  The annoyance is in the progress display.

In index-pack.c, fill() calls xread() and then display_throughput().
Since xread() is designed to call read(2) and simple continue on any
EINTR or EAGAIN, then — even though xread() explicitly does not
guarantee that ‘len’ bytes are read even if the data are available —
in practice xread() fills its buffer.  (At least on 32-bit x86,
using Linus’ kernel.)

Therefore, in practice — and as I have witnessed several thousand times
without ever having seen a contrary example — display_throughput() is
called *durring* a download only when total & 0xFFF == 0xFFF.

Perhaps, then, display_throughput() should round differently, so that
the logical equivilent of:

         ( ( n << 10) & 0x3FF ) / 1024.0

would be rounded up.  Then throughput_string() could elide the ".%2.2u"
whenever ((int)(total & ((1 << 10) - 1)) * 100) >> 10) == 0.

Or throughput_string() could simply elide the ".%2.2u" whenever
total & 0x3FF == 0x3FF.

Nicolas> I must NACK your patches.  Presumptions are not good enough
Nicolas> justification for such a change, especially if results can't
Nicolas> be reproduced.

Understood.  I concentrated on the progress display and ignored the
final display.

-JimC
-- 
James Cloos <[email-masked]>         OpenPGP: 1024D/ED7DAEA6