[PATCH v17 0/4] MR852: ntdll: Speed up NtQueryVirtualMemory(MemoryWorkingSetExInformation) by conditional pread.

[Witold Baryluk (＠baryluk)]

Apex Legends game periodically (every 30 seconds) calls this function
with up to 22k virtual addresses. All but 1 of them is valid. Due to
amount of queries addresses, and cost of seek+read, this causes this
function to take up to about 50ms. So framerate drops from ~150 FPS to
20FPS for about a second.

As far as I can see, returning 0 entries from this function, still makes
Apex Legend work.

But keep code correct, and optimise it by:

1. Opening pagemap file once, and never closing it. This eliminates
repeated fopen/fseek/fread/fclose sequences, which helps even in queries
with small amount of virtual addresses.

2. Using pread, instead of seek+read.

3. Only performing pagemap read when the address is valid.

Future work might recognize continues pages in the query, and perform a
batch read of multiple pagemap entries, instead one page at a time, but
for now it is not necassary.

This change get_working_set_ex peek wall clock runtime from 57ms to
0.29ms.

Tested on Linux, but similar change done for the BSD part.

`Signed-off-by: Witold Baryluk `

--
  v17: ntdll: Keep pagemap file open after first use of NtQueryVirtualMemory(MemoryWorkingSetExInformation)

https://gitlab.winehq.org/wine/wine/-/merge_requests/852

[PATCH v17 1/4] ntdll: Speed up NtQueryVirtualMemory(MemoryWorkingSetExInformation) by conditional page check

[Witold Baryluk]

From: Witold Baryluk

Legends game periodically (every 30 seconds) calls this function
with up to 22k virtual addresses. All but 1 of them is valid. Due to
amount of queries addresses, and cost of seek+read, this causes this
function to take up to about 50ms. So framerate drops from ~150 FPS to
20FPS for about a second.

As far as I can see, returning 0 entries from this function, still makes
Apex Legend work.

But keep code correct, and optimise it by only performing pagemap read
when the address is valid.

This change get_working_set_ex reduces peek wall clock runtime from 57ms
to 0.29ms.

Tested on Linux, but similar change done for the BSD part.

Signed-off-by: Witold Baryluk 
---
 dlls/ntdll/unix/virtual.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c
index e0aa410373e..bf9af40ffe6 100644
--- a/dlls/ntdll/unix/virtual.c
+++ b/dlls/ntdll/unix/virtual.c
@@ -4237,16 +4237,17 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
              BYTE vprot;
              struct file_view *view;

-             for (i = 0; i < vmentry_count && entry == NULL; i++)
-             {
-                 if (vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= vmentries[i].kve_end)
-                     entry = &vmentries[i];
-             }
              memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) );
              if ((view = find_view( p->VirtualAddress, 0 )) &&
                  get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) &&
                  (vprot & VPROT_COMMITTED))
              {
+                 for (i = 0; i < vmentry_count && entry == NULL; i++)
+                 {
+                     if (vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= vmentries[i].kve_end)
+                         entry = &vmentries[i];
+                 }
+
                  p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP;
                  p->VirtualAttributes.Shared = !is_view_valloc( view );
                  if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid)
@@ -4281,17 +4282,17 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,

memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) );

-        /* If we don't have pagemap information, default to invalid. */
-        if (!f || fseek( f, ((UINT_PTR)p->VirtualAddress >> 12) * sizeof(pagemap), SEEK_SET ) == -1 ||
-                fread( &pagemap, sizeof(pagemap), 1, f ) != 1)
-        {
-            pagemap = 0;
-        }
-
         if ((view = find_view( p->VirtualAddress, 0 )) &&
             get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) &&
             (vprot & VPROT_COMMITTED))
         {
+            if (!f || fseek( f, ((UINT_PTR)p->VirtualAddress >> 12) * sizeof(pagemap), SEEK_SET ) == -1 ||
+                    fread( &pagemap, sizeof(pagemap), 1, f ) != 1)
+            {
+                /* If we don't have pagemap information, default to invalid. */
+                pagemap = 0;
+            }
+
             p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63);
             p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1);
             if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid)

[PATCH v17 2/4] ntdll: Do not use hardcoded page shift in NtQueryVirtualMemory(MemoryWorkingSetExInformation)

[Witold Baryluk]

From: Witold Baryluk

---
 dlls/ntdll/unix/virtual.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c
index bf9af40ffe6..5c7370b6d94 100644
--- a/dlls/ntdll/unix/virtual.c
+++ b/dlls/ntdll/unix/virtual.c
@@ -4286,7 +4286,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
             get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) &&
             (vprot & VPROT_COMMITTED))
         {
-            if (!f || fseek( f, ((UINT_PTR)p->VirtualAddress >> 12) * sizeof(pagemap), SEEK_SET ) == -1 ||
+            if (!f || fseek( f, ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap), SEEK_SET ) == -1 ||
                     fread( &pagemap, sizeof(pagemap), 1, f ) != 1)
             {
                 /* If we don't have pagemap information, default to invalid. */

[PATCH v17 3/4] ntdll: Use pread in NtQueryVirtualMemory(MemoryWorkingSetExInformation)

[Witold Baryluk]

From: Witold Baryluk

1 syscall instead of 2 syscalls. Faster and simpler code.
---
 dlls/ntdll/unix/virtual.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c
index 5c7370b6d94..58afc87375c 100644
--- a/dlls/ntdll/unix/virtual.c
+++ b/dlls/ntdll/unix/virtual.c
@@ -4203,7 +4203,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
                                     MEMORY_WORKING_SET_EX_INFORMATION *info,
                                     SIZE_T len, SIZE_T *res_len )
 {
-    FILE *f = NULL;
+    int pagemap_fd;
     MEMORY_WORKING_SET_EX_INFORMATION *p;
     sigset_t sigset;

@@ -4266,8 +4266,8 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
             procstat_close( pstat );
     }
 #else
-    f = fopen( "/proc/self/pagemap", "rb" );
-    if (!f)
+    pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 );
+    if (pagemap_fd == -1)
     {
         static int once;
         if (!once++) WARN( "unable to open /proc/self/pagemap\n" );
@@ -4286,8 +4286,8 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
             get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) &&
             (vprot & VPROT_COMMITTED))
         {
-            if (!f || fseek( f, ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap), SEEK_SET ) == -1 ||
-                    fread( &pagemap, sizeof(pagemap), 1, f ) != 1)
+            if (pagemap_fd == -1 ||
+                pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap))
             {
                 /* If we don't have pagemap information, default to invalid. */
                 pagemap = 0;
@@ -4304,8 +4304,8 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
     server_leave_uninterrupted_section( &virtual_mutex, &sigset );
 #endif

-    if (f)
-        fclose( f );
+    if (pagemap_fd != -1)
+        close( pagemap_fd );
     if (res_len)
         *res_len = (UINT_PTR)p - (UINT_PTR)info;
     return STATUS_SUCCESS;

[PATCH v17 4/4] ntdll: Keep pagemap file open after first use of NtQueryVirtualMemory(MemoryWorkingSetExInformation)

[Witold Baryluk]

From: Witold Baryluk

---
 dlls/ntdll/unix/virtual.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c
index 58afc87375c..d8b888aa49e 100644
--- a/dlls/ntdll/unix/virtual.c
+++ b/dlls/ntdll/unix/virtual.c
@@ -4203,7 +4203,9 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
                                     MEMORY_WORKING_SET_EX_INFORMATION *info,
                                     SIZE_T len, SIZE_T *res_len )
 {
-    int pagemap_fd;
+#if !defined(HAVE_LIBPROCSTAT)
+    static int pagemap_fd = -2;
+#endif
     MEMORY_WORKING_SET_EX_INFORMATION *p;
     sigset_t sigset;

@@ -4266,14 +4268,18 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
             procstat_close( pstat );
     }
 #else
-    pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 );
-    if (pagemap_fd == -1)
+    server_enter_uninterrupted_section( &virtual_mutex, &sigset );
+    if (pagemap_fd == -2)
     {
-        static int once;
-        if (!once++) WARN( "unable to open /proc/self/pagemap\n" );
+#ifdef O_CLOEXEC
+        if ((pagemap_fd = open( "/proc/self/pagemap", O_RDONLY | O_CLOEXEC, 0 )) == -1 && errno == EINVAL)
+#endif
+            pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 );
+
+        if (pagemap_fd == -1) WARN( "unable to open /proc/self/pagemap\n" );
+        else fcntl(pagemap_fd, F_SETFD, FD_CLOEXEC);  /* in case O_CLOEXEC isn't supported */
     }

-    server_enter_uninterrupted_section( &virtual_mutex, &sigset );
     for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++)
     {
         BYTE vprot;
@@ -4304,8 +4310,6 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr,
     server_leave_uninterrupted_section( &virtual_mutex, &sigset );
 #endif

-    if (pagemap_fd != -1)
-        close( pagemap_fd );
     if (res_len)
         *res_len = (UINT_PTR)p - (UINT_PTR)info;
     return STATUS_SUCCESS;

[Witold Baryluk (＠baryluk)]

On Wed Sep 21 21:43:32 2022 +0000, **** wrote:
> Marvin replied on the mailing list:
> Hi,
> It looks like your patch introduced the new failures shown below.
> Please investigate and fix them before resubmitting your patch.
> If they are not new, fixing them anyway would help a lot. Otherwise
> please ask for the known failures list to be updated.
> The full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=123962
> Your paranoid android.
> === debian11 (build log) ===
> ../wine/dlls/ntdll/unix/virtual.c:4280:9: error: expected ???;??? before ???else???
> Task: The win32 Wine build failed
> === debian11 (build log) ===
> ../wine/dlls/ntdll/unix/virtual.c:4280:9: error: expected ???;??? before ???else???
> Task: The wow64 Wine build failed

Too much Python today. Fixed.