Re: Performance regression in cygwin 3.4.0

[Takashi Yano <takashi.yano@nifty.ne.jp>]

[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]

Hi, Corinna, Ken, and other cygwin developers,

I tried to fix the following performance problem of
non-cygwin pipe and succeeded by applying the patch
attached.

Although it is not smart enough, it works as far as
I tested.

What do you think of this patch? Any other idea?

Comments or suggestions will be appreciated.

On Thu, 8 Dec 2022 18:46:56 +0900
Takashi Yano wrote:
> On Wed, 07 Dec 2022 19:38:19 +0000
> tryandbuy wrote:
> > Reading from stdin is very slow when a process runs outside of Cygwin Terminal.
> > 
> > Steps to reproduce:
> > 1. Create "test.txt" file using python code:
> > long_text = "10" * 2500
> > with open(r'test.txt', 'w') as f:
> >     for i in range(5000):
> >         f.write('KEY%03d: %d => %s\n' % (i % 100, i, long_text))
> > 
> > 2. Install "cygwin" package version > "3.3.6-1"
> > 3. Open windows command prompt (cmd.exe)
> > 4. Enter command:
> > type test.txt | c:\cygwin64\bin\wc.exe -l
> > 
> > When running the same command (use cat instead of type) on the Cygwin Terminal, no performance issues observed.
> > 
> > The last version of cygwin package without issues: 3.3.6-1
> > Versions of cygwin package with issues: 3.4.x - 3.5.x
> > 
> > P.S. I tested this combinations of pipings:
> > cygwin_prog | win_prog    # no issues
> > win_prog | cygwin_prog    # has issues when running in windows command prompt, no issues in Cygwin Terminal
> > cygwin_prog | cygwin_prog # has issues  when running in windows command prompt, no issues in Cygwin Terminal
> 
> I confirmed the problem. I also identified the code difference,
> which causes the problem, between cygwin 3.3.x and 3.4.0.
> 
> Let us consider how to fix that.

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

[-- Attachment #2: 0001-Cygwin-pipe-Fix-performance-degradation-for-non-cygw.patch --]
[-- Type: application/octet-stream, Size: 2681 bytes --]

From eb11d098dd8d0bf9c52291c5b5106715d5e3128d Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Thu, 8 Dec 2022 22:02:43 +0900
Subject: [PATCH] Cygwin: pipe: Fix performance degradation for non-cygwin
 pipe.

- After the commit 9e4d308cd592fe383dec58ea6523c1b436888ef8, the
  performance of read from non-cygwin pipe has been degraded. This
  is because select_sem mechanism does not work for non-cygwin pipe.
  This patch fixes the issue.

  Addresses:
    https://cygwin.com/pipermail/cygwin/2022-December/252628.html
---
 winsup/cygwin/fhandler/pipe.cc | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc
index 720e4efd3..654e94ebd 100644
--- a/winsup/cygwin/fhandler/pipe.cc
+++ b/winsup/cygwin/fhandler/pipe.cc
@@ -281,6 +281,9 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
   size_t nbytes = 0;
   NTSTATUS status = STATUS_SUCCESS;
   IO_STATUS_BLOCK io;
+  DWORD t0 = GetTickCount (); /* Init timer */
+  const DWORD t0_threshold = 20;
+  const DWORD t0_ovr = 100;
 
   if (!len)
     return;
@@ -312,6 +315,7 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
     {
       ULONG_PTR nbytes_now = 0;
       ULONG len1 = (ULONG) (len - nbytes);
+      DWORD select_sem_timeout = 0;
 
       FILE_PIPE_LOCAL_INFORMATION fpli;
       status = NtQueryInformationFile (get_handle (), &io,
@@ -338,6 +342,7 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
 	  nbytes += nbytes_now;
 	  if (select_sem && nbytes_now > 0)
 	    release_select_sem ("raw_read");
+	  t0 = GetTickCount (); /* Reset timer */
 	}
       else
 	{
@@ -358,7 +363,21 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
 		  nbytes = (size_t) -1;
 		  break;
 		}
-	      waitret = cygwait (select_sem, 1);
+	      /* Prevent timer from overflow */
+	      if (GetTickCount () - t0 > t0_ovr)
+		t0 += t0_ovr - t0_threshold;
+	      /* If the pipe is a non-cygwin pipe, select_sem trick
+		 does not work. As a result, the following cygwait()
+		 will return only after timeout occurs. This causes
+		 performance degradation. However, setting timeout
+		 to zero causes high CPU load. So, set timeout to
+		 non-zero only when select_sem is valid or pipe is
+		 not ready to read for more than t0_threshold.
+		 This prevents both the performance degradation and
+		 the high CPU load. */
+	      if (select_sem || GetTickCount () - t0 > t0_threshold)
+		select_sem_timeout = 1;
+	      waitret = cygwait (select_sem, select_sem_timeout);
 	      if (waitret == WAIT_CANCELED)
 		pthread::static_cancel_self ();
 	      else if (waitret == WAIT_SIGNALED)
-- 
2.38.1