[PATCH v5 0/4] MR852: ntdll: Speed up NtQueryVirtualMemory(MemoryWorkingSetExInformation) by conditional pread.
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 <witold.baryluk(a)gmail.com>` -- v5: ntdll: Keep pagemap file open after first use of NtQueryVirtualMemory(MemoryWorkingSetExInformation) https://gitlab.winehq.org/wine/wine/-/merge_requests/852
From: Witold Baryluk <witold.baryluk(a)gmail.com> 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 <witold.baryluk(a)gmail.com> --- 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 96a5e095d16..d4987c16e62 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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/852
From: Witold Baryluk <witold.baryluk(a)gmail.com> --- 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 d4987c16e62..adcd428d5a4 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. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/852
From: Witold Baryluk <witold.baryluk(a)gmail.com> 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 adcd428d5a4..31faae872af 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/852
From: Witold Baryluk <witold.baryluk(a)gmail.com> --- dlls/ntdll/unix/virtual.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 31faae872af..d67fd66bc9f 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -199,6 +199,8 @@ static BYTE **pages_vprot; static BYTE *pages_vprot; #endif +static int pagemap_fd = -1; + static struct file_view *view_block_start, *view_block_end, *next_free_view; static const size_t view_block_size = 0x100000; static void *preload_reserve_start; @@ -4203,7 +4205,6 @@ 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; MEMORY_WORKING_SET_EX_INFORMATION *p; sigset_t sigset; @@ -4266,14 +4267,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 ); { static int once; if (!once++) WARN( "unable to open /proc/self/pagemap\n" ); + static int once = 0; + if (once == 0 && pagemap_fd == -1) { + pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 ); + if (pagemap_fd == -1) WARN( "unable to open /proc/self/pagemap\n" ); + once = 1; + } } - server_enter_uninterrupted_section( &virtual_mutex, &sigset ); for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) { BYTE vprot; @@ -4304,8 +4309,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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/852
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=123470 Your paranoid android. === debian11 (build log) === ../wine/dlls/ntdll/unix/virtual.c:4274:20: error: redeclaration of ���once��� with no linkage Task: The win32 Wine build failed === debian11 (build log) === ../wine/dlls/ntdll/unix/virtual.c:4274:20: error: redeclaration of ���once��� with no linkage Task: The wow64 Wine build failed
participants (3)
-
Marvin -
Witold Baryluk -
Witold Baryluk (＠baryluk)