[PATCH v2] Cygwin: pty: Revise code waiting for forwarding by master_fwd_thread.

Takashi Yano takashi.yano@nifty.ne.jp
Wed Jan 22 16:00:00 GMT 2020


Hi Corinna,

On Tue, 21 Jan 2020 10:37:35 +0100
Corinna Vinschen wrote:
> On Jan 21 11:22, Takashi Yano wrote:
> > - Though this rarely happens, sometimes the first printing of non-
> >   cygwin process does not displayed correctly. To fix this issue,
> >   the code for waiting for forwarding by master_fwd_thread is revised.
> 
> Looks good.  Pushed.

First of all, I have to apologize for insufficient test.
With this patch, the following test case frequently displays
its output incorrectly.

Sometimes it is just
AAAA
or
AAAA
AAAA (duplicated)
or
AAAA
<blank line>

Of course, the expected ersult is:
AAAA
BBBB

/* Test code */
#include <windows.h>
#include <stdio.h>

int main()
{
    DWORD n;
    printf("AAAA\n");
    WriteConsole(GetStdHandle(STD_OUTPUT_HANDLE), "BBBB\r\n", 6, &n, 0);
    return 0;
}

I looked into this problem and found that the cause. What we
really need is not waiting for forwarding by pty_master_fwd_thread
but is waiting for forwarding by pseudo console itself. In other
words, the latency between slave write to get_output_handle()
and master read from from_slave pipe is dominant.

We cannot touch the forwarding process in pseudo console, therefore
the strategy like this patch can not be used. So, we might have to
revert to dumb Sleep() wait.

As for the wait time, I have measured the latency of pseudo console
in following machines.

Machine1: Ryzen 9 3950X (3.5GHz)
Machine2: Core i7 6700K (4.0GHz)
Machine3: Xeon X5670 (2.93GHz)
Machine4: Core i7 870 (2.93GHz)
Machine5: PentinumD 930 (3.0GHz)  <- Very old!

Measuring is done 1000 times for each machine. The results are as
follows.

Machine1:
Count  Latency[ms]
  416   0
  222  15
  362  16

Machine2:
Count  Latency[ms]
  421   0
  185  15
  394  16

Machine3:
Count  Latency[ms]
  439   0
  199  15
  362  16

Machine4:
Count  Latency[ms]
  202  15
  340  16
  343  31
  115  32

Machine5:
Count  Latency[ms]
  140 15
  284 16
  410 31
  159 32
    7 47

Obviously, the latency depends on the machine performance.
Therefore, I will propose new patch which uses dumb Sleep()
with on-the-fly performance test.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>



More information about the Cygwin-patches mailing list