Re: [PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait

[Junio C Hamano <gitster@pobox.com>]

Stepan Kasal <kasal@ucw.cz> writes:

> Hello Junio,
>
> thank you for pointing out the problems.
>
> Let me explain the background:
>
> After some discussion a one line fix to win32/poll.c was accepted to msysgit/git
> at 2012-05-16 https://github.com/msysgit/git/pull/7
>
> The description of the commit looked like this:
>> I played around with this [...]
>> [...]
>> I also tested the very fast local case, and didn't see any measurable
>> difference. On a big repo with 4500 files, the upload-pack took about 2
>> seconds with and without the fix.
> ... but there was no sign-off by Theodore.
>
> Because poll.c comes from Gnulib, it was reported there as well.
> Paolo Bonzini accepted the fix for poll.c and added a fix for select.
> The combined commit got this changelog entry:
>
>> 2012-05-21  Paolo Bonzini  <bonzini@gnu.org>
>> 
>>         poll/select: prevent busy-waiting.  SwitchToThread() only gives away
>>         the rest of the current time slice to another thread in the current
>>         process. So if the thread that feeds the file decscriptor we're
>>         polling is not in the current process, we get busy-waiting.
>>         * lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().
>>         Patch from Theodore Leblond.
>>         * lib/select.c: Split polling out of the loop that sets the output
>>         fd_sets.  Check for zero result and loop if the wait timeout is
>>         infinite.
>
> The patch that I (Stepan) submitted as "v2" combines things like this:
> - subject by Theodore
> - first paragraph by Paolo, concise problem description
> - rest from Theodore's original commit ("I" = Theodore, I suppose)
> - diff exctly as in gnulib commit
>
> On Mon, Apr 28, 2014 at 11:58:50AM -0700, Junio C Hamano wrote:
>>     Subject: compat/poll: sleep 1 millisecond to avoid busy wait
>
> Thanks for improving it.
>
>>     Signed-off-by: Theodore Leblond <theodore.leblond@gmail.com>
>>     Signed-off-by: Stepan Kasal <kasal@ucw.cz>
>>     Acked-by: Johannes Sixt <j6t@kdbg.org>
>>     Acked-by: Erik Faye-Lund <kusmabite@gmail.com>
>
> Sorry that I forgot to add my sign-off (Stepan).
> But I'm afraid I cannot add Theodore's, it was not in the original
> commit.  But it is one line change; the real work was the analysis.

Well, the original git/pull/7 in msysgit repository says that is a
patch by Theodore, so the kosher thing to do is to ask him (and I
see he is on the Cc list), and also ask msysgit folks (and I see
they are on the Cc list as well) to be a bit more careful when
responding to pull requests, especially given that it is our mutual
benefit to make sure we keep the changes between our two trees to
the minimum by upstreaming.

I'll queue without the forged sign-off by Theodore, as I think DCO
(1.1) (c) read loosely would let me do so ;-)

Thanks.