* [PATCH] Cygwin: pipe: Fix performance degradation for non-cygwin pipe.
@ 2022-12-09 0:49 Takashi Yano
2022-12-09 9:59 ` Corinna Vinschen
0 siblings, 1 reply; 2+ messages in thread
From: Takashi Yano @ 2022-12-09 0:49 UTC (permalink / raw)
To: cygwin-patches; +Cc: Takashi Yano, tryandbuy, Corinna Vinschen
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 <tryandbuy@proton.me>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Cygwin: pipe: Fix performance degradation for non-cygwin pipe.
2022-12-09 0:49 [PATCH] Cygwin: pipe: Fix performance degradation for non-cygwin pipe Takashi Yano
@ 2022-12-09 9:59 ` Corinna Vinschen
0 siblings, 0 replies; 2+ messages in thread
From: Corinna Vinschen @ 2022-12-09 9:59 UTC (permalink / raw)
To: cygwin-patches
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 <tryandbuy@proton.me>
> Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> ---
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-12-09 9:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 0:49 [PATCH] Cygwin: pipe: Fix performance degradation for non-cygwin pipe Takashi Yano
2022-12-09 9:59 ` Corinna Vinschen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).