Re: cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

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

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

On Thu, 23 Sep 2021 09:03:13 -0400
Ken Brown wrote:
> On 9/23/2021 4:26 AM, Takashi Yano wrote:
> > * Make temporary_query_hdl() faster.
> 
> Thanks!  Again, I'm not familiar with the Windows functions that you used, so 
> Corinna should review this too when she returns.
> 
> I have one question:
> 
> The documentation at 
> https://docs.microsoft.com/en-us/windows/win32/api/psapi/nf-psapi-enumprocesses 
> says, "There is no indication given when the buffer is too small to store all 
> process identifiers. Therefore, if lpcbNeeded equals cb, consider retrying the 
> call with a larger array."
> 
> Does this mean that EnumProcesses could return TRUE even if the buffer is too small?

Ah, right. Thanks for pointing out that mistake.
I have fixed that.

P.S.
I measured the response of select().
The first time and second time response is measured.

Only the first time takes long response time because
the cached info is used in the second time. This patch
improves the response much. Also, caching info is very
effective.

First time, Second time
Without this patch:
19.427600 [msec], 0.113900 [msec]

With this patch:
3.344600 [msec], 0.115000 [msec]

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

[-- Attachment #2: v2-0001-Cygwin-pipe-Use-ProcessHandleInformation-if-avail.patch --]
[-- Type: application/octet-stream, Size: 11858 bytes --]

From 11741ec6de92022ec30bd57a45b6d66f7cb8ebe0 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Thu, 23 Sep 2021 23:33:55 +0900
Subject: [PATCH v2] Cygwin: pipe: Use ProcessHandleInformation if available.

- The commit b531d6b0 introduced temporary_query_hdl() which uses
  SystemHandleInformation. With this patch, ProcessHandleInformation
  rather than SystemHandleInformation is used if it is available.
  This request is faster, however, is only available since Windows 8,
  therefore, SystemHandleInformation is used for Windows Vista and 7
  as before.
---
 winsup/cygwin/fhandler.h       |   2 +
 winsup/cygwin/fhandler_pipe.cc | 133 +++++++++++++++++++++++++++++++--
 winsup/cygwin/ntdll.h          |  21 +++++-
 winsup/cygwin/wincap.cc        |  11 +++
 winsup/cygwin/wincap.h         |   2 +
 5 files changed, 160 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0061d4830..c033f7816 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1197,6 +1197,8 @@ private:
   DWORD pipename_pid;
   LONG pipename_id;
   void release_select_sem (const char *);
+  HANDLE get_query_hdl_per_process (WCHAR *, OBJECT_NAME_INFORMATION *);
+  HANDLE get_query_hdl_per_system (WCHAR *, OBJECT_NAME_INFORMATION *);
 public:
   fhandler_pipe ();
 
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index 78e2f90d8..dd8b4f317 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -20,6 +20,7 @@ details. */
 #include "pinfo.h"
 #include "shared_info.h"
 #include "tls_pbuf.h"
+#include <psapi.h>
 
 /* This is only to be used for writing.  When reading,
 STATUS_PIPE_EMPTY simply means there's no data to be read. */
@@ -1176,14 +1177,129 @@ cache_err:
 	       &pipename_key, &pipename_pid, &pipename_id) != 3)
     return NULL; /* Non cygwin pipe? */
 
+  if (wincap.has_query_process_handle_info ())
+    return get_query_hdl_per_process (name, ntfn); /* Since Win8 */
+  else
+    return get_query_hdl_per_system (name, ntfn); /* Vista or Win7 */
+}
+
+/* This function is faster than get_query_hdl_per_system(), however,
+   only works since Windows 8 because ProcessHandleInformation is not
+   suppoted by NtQueryInformationProcess() before Windows 8. */
+HANDLE
+fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
+					  OBJECT_NAME_INFORMATION *ntfn)
+{
+  ULONG len;
+  BOOL res;
+  DWORD n_process = 256;
+  DWORD *proc_pids;
+  do
+    { /* Enumerate processes */
+      DWORD nbytes = n_process * sizeof (DWORD);
+      proc_pids = (DWORD *) HeapAlloc (GetProcessHeap (), 0, nbytes);
+      if (!proc_pids)
+	return NULL;
+      res = EnumProcesses (proc_pids, nbytes, &len);
+      if (res && len < nbytes)
+	break;
+      res = FALSE;
+      HeapFree (GetProcessHeap (), 0, proc_pids);
+      n_process *= 2;
+    }
+  while (n_process < (1L<<20));
+  if (!res)
+    return NULL;
+  n_process = len / sizeof (DWORD);
+
+  for (LONG i = (LONG) n_process - 1; i >= 0; i--)
+    {
+      HANDLE proc = OpenProcess (PROCESS_DUP_HANDLE
+				 | PROCESS_QUERY_INFORMATION,
+				 0, proc_pids[i]);
+      if (!proc)
+	continue;
+
+      /* Retrieve process handles */
+      NTSTATUS status;
+      DWORD n_handle = 256;
+      PPROCESS_HANDLE_SNAPSHOT_INFORMATION phi;
+      do
+	{
+	  DWORD nbytes = 2 * sizeof (ULONG_PTR) +
+	    n_handle * sizeof (PROCESS_HANDLE_TABLE_ENTRY_INFO);
+	  phi = (PPROCESS_HANDLE_SNAPSHOT_INFORMATION)
+	    HeapAlloc (GetProcessHeap (), 0, nbytes);
+	  if (!phi)
+	    goto close_proc;
+	  status = NtQueryInformationProcess (proc, ProcessHandleInformation,
+					      phi, nbytes, &len);
+	  if (NT_SUCCESS (status))
+	    break;
+	  HeapFree (GetProcessHeap (), 0, phi);
+	  n_handle *= 2;
+	}
+      while (n_handle < (1L<<20) && status == STATUS_INFO_LENGTH_MISMATCH);
+      if (!NT_SUCCESS (status))
+	goto close_proc;
+
+      for (ULONG j = 0; j < phi->NumberOfHandles; j++)
+	{
+	  /* Check for the peculiarity of cygwin read pipe */
+	  const ULONG access = FILE_READ_DATA | FILE_READ_EA
+	    | FILE_WRITE_EA /* marker */
+	    | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
+	    | READ_CONTROL | SYNCHRONIZE;
+	  if (phi->Handles[j].GrantedAccess != access)
+	    continue;
+
+	  /* Retrieve handle */
+	  HANDLE h = (HANDLE)(intptr_t) phi->Handles[j].HandleValue;
+	  res = DuplicateHandle (proc, h, GetCurrentProcess (), &h,
+				 FILE_READ_DATA, 0, 0);
+	  if (!res)
+	    continue;
+
+	  /* Check object name */
+	  status = NtQueryObject (h, ObjectNameInformation,
+				  ntfn, 65536, &len);
+	  if (!NT_SUCCESS (status) || !ntfn->Name.Buffer)
+	    goto close_handle;
+	  ntfn->Name.Buffer[ntfn->Name.Length / sizeof (WCHAR)] = L'\0';
+	  if (wcscmp (name, ntfn->Name.Buffer) == 0)
+	    {
+	      query_hdl_proc = proc;
+	      query_hdl_value = (HANDLE)(intptr_t) phi->Handles[j].HandleValue;
+	      HeapFree (GetProcessHeap (), 0, phi);
+	      HeapFree (GetProcessHeap (), 0, proc_pids);
+	      return h;
+	    }
+close_handle:
+	  CloseHandle (h);
+	}
+      HeapFree (GetProcessHeap (), 0, phi);
+close_proc:
+      CloseHandle (proc);
+    }
+  HeapFree (GetProcessHeap (), 0, proc_pids);
+  return NULL;
+}
+
+/* This function is slower than get_query_hdl_per_process(), however,
+   works even before Windows 8. */
+HANDLE
+fhandler_pipe::get_query_hdl_per_system (WCHAR *name,
+					 OBJECT_NAME_INFORMATION *ntfn)
+{
+  NTSTATUS status;
   SIZE_T n_handle = 65536;
   PSYSTEM_HANDLE_INFORMATION shi;
   do
-    {
+    { /* Enumerate handles */
       SIZE_T nbytes =
 	sizeof (ULONG) + n_handle * sizeof (SYSTEM_HANDLE_TABLE_ENTRY_INFO);
       shi = (PSYSTEM_HANDLE_INFORMATION) HeapAlloc (GetProcessHeap (),
-						     0, nbytes);
+						    0, nbytes);
       if (!shi)
 	return NULL;
       status = NtQuerySystemInformation (SystemHandleInformation,
@@ -1193,15 +1309,15 @@ cache_err:
       HeapFree (GetProcessHeap (), 0, shi);
       n_handle *= 2;
     }
-  while (n_handle < (1L<<20));
+  while (n_handle < (1L<<23) && status == STATUS_INFO_LENGTH_MISMATCH);
   if (!NT_SUCCESS (status))
     return NULL;
 
-  HANDLE qh = NULL;
   for (LONG i = (LONG) shi->NumberOfHandles - 1; i >= 0; i--)
     {
       /* Check for the peculiarity of cygwin read pipe */
-      DWORD access = FILE_READ_DATA | FILE_READ_EA | FILE_WRITE_EA /* marker */
+      const ULONG access = FILE_READ_DATA | FILE_READ_EA
+	| FILE_WRITE_EA /* marker */
 	| FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
 	| READ_CONTROL | SYNCHRONIZE;
       if (shi->Handles[i].GrantedAccess != access)
@@ -1219,6 +1335,7 @@ cache_err:
 	goto close_proc;
 
       /* Check object name */
+      ULONG len;
       status = NtQueryObject (h, ObjectNameInformation, ntfn, 65536, &len);
       if (!NT_SUCCESS (status) || !ntfn->Name.Buffer)
 	goto close_handle;
@@ -1227,8 +1344,8 @@ cache_err:
 	{
 	  query_hdl_proc = proc;
 	  query_hdl_value = (HANDLE)(intptr_t) shi->Handles[i].HandleValue;
-	  qh = h;
-	  break;
+	  HeapFree (GetProcessHeap (), 0, shi);
+	  return h;
 	}
 close_handle:
       CloseHandle (h);
@@ -1236,5 +1353,5 @@ close_proc:
       CloseHandle (proc);
     }
   HeapFree (GetProcessHeap (), 0, shi);
-  return qh;
+  return NULL;
 }
diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h
index e8c3c45c5..0510d833b 100644
--- a/winsup/cygwin/ntdll.h
+++ b/winsup/cygwin/ntdll.h
@@ -842,9 +842,28 @@ typedef enum _PROCESSINFOCLASS
   ProcessSessionInformation = 24,
   ProcessWow64Information = 26,
   ProcessImageFileName = 27,
-  ProcessDebugFlags = 31
+  ProcessDebugFlags = 31,
+  ProcessHandleInformation = 51 /* Since Win8 */
 } PROCESSINFOCLASS;
 
+typedef struct _PROCESS_HANDLE_TABLE_ENTRY_INFO
+{
+  HANDLE HandleValue;
+  ULONG_PTR HandleCount;
+  ULONG_PTR PointerCount;
+  ULONG GrantedAccess;
+  ULONG ObjectTypeIndex;
+  ULONG HandleAttributes;
+  ULONG Reserved;
+} PROCESS_HANDLE_TABLE_ENTRY_INFO, *PPROCESS_HANDLE_TABLE_ENTRY_INFO;
+
+typedef struct _PROCESS_HANDLE_SNAPSHOT_INFORMATION
+{
+  ULONG_PTR NumberOfHandles;
+  ULONG_PTR Reserved;
+  PROCESS_HANDLE_TABLE_ENTRY_INFO Handles[1];
+} PROCESS_HANDLE_SNAPSHOT_INFORMATION, *PPROCESS_HANDLE_SNAPSHOT_INFORMATION;
+
 typedef struct _DEBUG_BUFFER
 {
   HANDLE SectionHandle;
diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index 635e0892b..6c79d8710 100644
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -50,6 +50,7 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
     has_tcp_fastopen:false,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:false,
+    has_query_process_handle_info:false,
   },
 };
 
@@ -85,6 +86,7 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
     has_tcp_fastopen:false,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:false,
+    has_query_process_handle_info:false,
   },
 };
 
@@ -120,6 +122,7 @@ wincaps wincap_8 __attribute__((section (".cygwin_dll_common"), shared)) = {
     has_tcp_fastopen:false,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:false,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -155,6 +158,7 @@ wincaps wincap_8_1 __attribute__((section (".cygwin_dll_common"), shared)) = {
     has_tcp_fastopen:false,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:false,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -190,6 +194,7 @@ wincaps  wincap_10_1507 __attribute__((section (".cygwin_dll_common"), shared))
     has_tcp_fastopen:false,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:false,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -225,6 +230,7 @@ wincaps  wincap_10_1607 __attribute__((section (".cygwin_dll_common"), shared))
     has_tcp_fastopen:true,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:true,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -260,6 +266,7 @@ wincaps wincap_10_1703 __attribute__((section (".cygwin_dll_common"), shared)) =
     has_tcp_fastopen:true,
     has_linux_tcp_keepalive_sockopts:false,
     has_tcp_maxrtms:true,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -295,6 +302,7 @@ wincaps wincap_10_1709 __attribute__((section (".cygwin_dll_common"), shared)) =
     has_tcp_fastopen:true,
     has_linux_tcp_keepalive_sockopts:true,
     has_tcp_maxrtms:true,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -330,6 +338,7 @@ wincaps wincap_10_1803 __attribute__((section (".cygwin_dll_common"), shared)) =
     has_tcp_fastopen:true,
     has_linux_tcp_keepalive_sockopts:true,
     has_tcp_maxrtms:true,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -365,6 +374,7 @@ wincaps wincap_10_1809 __attribute__((section (".cygwin_dll_common"), shared)) =
     has_tcp_fastopen:true,
     has_linux_tcp_keepalive_sockopts:true,
     has_tcp_maxrtms:true,
+    has_query_process_handle_info:true,
   },
 };
 
@@ -400,6 +410,7 @@ wincaps wincap_10_1903 __attribute__((section (".cygwin_dll_common"), shared)) =
     has_tcp_fastopen:true,
     has_linux_tcp_keepalive_sockopts:true,
     has_tcp_maxrtms:true,
+    has_query_process_handle_info:true,
   },
 };
 
diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h
index 6be2ca2a1..7249b9518 100644
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@@ -44,6 +44,7 @@ struct wincaps
     unsigned has_tcp_fastopen					: 1;
     unsigned has_linux_tcp_keepalive_sockopts			: 1;
     unsigned has_tcp_maxrtms					: 1;
+    unsigned has_query_process_handle_info			: 1;
   };
 };
 
@@ -111,6 +112,7 @@ public:
   bool	IMPLEMENT (has_tcp_fastopen)
   bool	IMPLEMENT (has_linux_tcp_keepalive_sockopts)
   bool	IMPLEMENT (has_tcp_maxrtms)
+  bool	IMPLEMENT (has_query_process_handle_info)
 
   void disable_case_sensitive_dirs ()
   {
-- 
2.33.0