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

From: Stepan Kasal <kasal@ucw.cz>
Date: Apr 29, 2014, 03:47 UTC
Message-ID: <20140429034727.GB15181@camelia.ucw.cz>
In-Reply-To: <xmqqfvkxqo0l.fsf@gitster.dls.corp.google.com>

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:

Show 5 quoted lines

> 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:

Show 11 quoted lines

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

Stepan