Hi Takashi, both patches LGTM, feel free to push. Your commit message looks great!
Thanks, Corinna On Dec 9 09:49, Takashi Yano wrote: > https://cygwin.com/pipermail/cygwin/2022-December/252628.html > > After the commit 9e4d308cd592, 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. > > Fixes: 9e4d308cd592 ("Cygwin: pipe: Adopt FILE_SYNCHRONOUS_IO_NONALERT > flag for read pipe.") > Reported-by: tryandbuy <[email protected]> > Reviewed-by: Corinna Vinschen <[email protected]> > Signed-off-by: Takashi Yano <[email protected]> > --- > winsup/cygwin/fhandler/pipe.cc | 16 +++++++++++++++- > winsup/cygwin/release/3.4.1 | 3 +++ > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc > index 720e4efd3..e23131668 100644 > --- a/winsup/cygwin/fhandler/pipe.cc > +++ b/winsup/cygwin/fhandler/pipe.cc > @@ -281,6 +281,8 @@ fhandler_pipe::raw_read (void *ptr, size_t& len) > size_t nbytes = 0; > NTSTATUS status = STATUS_SUCCESS; > IO_STATUS_BLOCK io; > + ULONGLONG t0 = GetTickCount64 (); /* Init timer */ > + const ULONGLONG t0_threshold = 20; > > if (!len) > return; > @@ -312,6 +314,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, > @@ -358,7 +361,18 @@ fhandler_pipe::raw_read (void *ptr, size_t& len) > nbytes = (size_t) -1; > break; > } > - waitret = cygwait (select_sem, 1); > + /* 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 || GetTickCount64 () - 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) > diff --git a/winsup/cygwin/release/3.4.1 b/winsup/cygwin/release/3.4.1 > index 432113a55..afe7e86f9 100644 > --- a/winsup/cygwin/release/3.4.1 > +++ b/winsup/cygwin/release/3.4.1 > @@ -8,3 +8,6 @@ Bug Fixes > - Fix a backward incompatibility problem in the definition of the > base type of the stdio type FILE. > Addresses: https://savannah.gnu.org/bugs/index.php?63480 > + > +- Fix performance degradation of non-cygwin pipe. > + Addresses: https://cygwin.com/pipermail/cygwin/2022-December/252628.html > -- > 2.38.1
