[PATCH 0/4] MR5970: win32u: Use the thread message queue shared memory in peek_message.
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/message.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 2f5ffc3a538..a391f733cdb 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2693,10 +2693,9 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) INPUT_MESSAGE_SOURCE prev_source = thread_info->client_info.msg_source; struct received_message_info info; unsigned int hw_id = 0; /* id of previous hardware message */ - void *buffer; - size_t buffer_size = 1024; - - if (!(buffer = malloc( buffer_size ))) return -1; + unsigned char buffer_init[1024]; + size_t buffer_size = sizeof(buffer_init); + void *buffer = buffer_init; if (!first && !last) last = ~0; if (hwnd == HWND_BROADCAST) hwnd = HWND_TOPMOST; @@ -2739,7 +2738,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) if (res) { - free( buffer ); + if (buffer != buffer_init) free( buffer ); if (res == STATUS_PENDING) { return 0; @@ -2766,6 +2765,12 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) break; case MSG_NOTIFY: info.flags = ISMEX_NOTIFY; + /* unpack_message may have to reallocate */ + if (buffer == buffer_init) + { + buffer = malloc( buffer_size ); + memcpy( buffer, buffer_init, buffer_size ); + } if (!unpack_message( info.msg.hwnd, info.msg.message, &info.msg.wParam, &info.msg.lParam, &buffer, size, &buffer_size )) continue; @@ -2844,6 +2849,12 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) continue; case MSG_OTHER_PROCESS: info.flags = ISMEX_SEND; + /* unpack_message may have to reallocate */ + if (buffer == buffer_init) + { + buffer = malloc( buffer_size ); + memcpy( buffer, buffer_init, buffer_size ); + } if (!unpack_message( info.msg.hwnd, info.msg.message, &info.msg.wParam, &info.msg.lParam, &buffer, size, &buffer_size )) { @@ -2866,7 +2877,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) thread_info->client_info.message_pos = MAKELONG( info.msg.pt.x, info.msg.pt.y ); thread_info->client_info.message_time = info.msg.time; thread_info->client_info.message_extra = msg_data->hardware.info; - free( buffer ); + if (buffer != buffer_init) free( buffer ); call_hooks( WH_GETMESSAGE, HC_ACTION, flags & PM_REMOVE, (LPARAM)msg, sizeof(*msg) ); return 1; } @@ -2881,7 +2892,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) /* if this is a nested call return right away */ if (first == info.msg.message && last == info.msg.message) { - free( buffer ); + if (buffer != buffer_init) free( buffer ); return 0; } } @@ -2930,7 +2941,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) thread_info->client_info.message_time = info.msg.time; thread_info->client_info.message_extra = 0; thread_info->client_info.msg_source = msg_source_unavailable; - free( buffer ); + if (buffer != buffer_init) free( buffer ); call_hooks( WH_GETMESSAGE, HC_ACTION, flags & PM_REMOVE, (LPARAM)msg, sizeof(*msg) ); return 1; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5970
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/message.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index a391f733cdb..c19837b3877 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2741,6 +2741,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) if (buffer != buffer_init) free( buffer ); if (res == STATUS_PENDING) { + NtYieldExecution(); return 0; } if (res != STATUS_BUFFER_OVERFLOW) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5970
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/message.c | 25 +++++++++++++++++++++++-- dlls/win32u/ntuser_private.h | 1 + server/queue.c | 6 ++++-- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index c19837b3877..1cb6e3189e6 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2705,10 +2705,30 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) NTSTATUS res; size_t size = 0; const message_data_t *msg_data = buffer; + UINT wake_mask, signal_bits, wake_bits, changed_bits, clear_bits = 0; + + /* use the same logic as in server/queue.c get_message */ + if (!(signal_bits = flags >> 16)) signal_bits = QS_ALLINPUT; + + if (signal_bits & QS_POSTMESSAGE) + { + clear_bits |= QS_POSTMESSAGE | QS_HOTKEY | QS_TIMER; + if (first == 0 && last == ~0U) clear_bits |= QS_ALLPOSTMESSAGE; + } + if (signal_bits & QS_INPUT) clear_bits |= QS_INPUT; + if (signal_bits & QS_PAINT) clear_bits |= QS_PAINT; + + /* if filter includes QS_RAWINPUT we have to translate hardware messages */ + if (signal_bits & QS_RAWINPUT) signal_bits |= QS_KEY | QS_MOUSEMOVE | QS_MOUSEBUTTON; thread_info->client_info.msg_source = prev_source; + wake_mask = filter->mask & (QS_SENDMESSAGE | QS_SMRESULT); - SERVER_START_REQ( get_message ) + if (NtGetTickCount() - thread_info->last_getmsg_time < 3000 && /* avoid hung queue */ + check_queue_bits( wake_mask, filter->mask, wake_mask | signal_bits, filter->mask | clear_bits, + &wake_bits, &changed_bits )) + res = STATUS_PENDING; + else SERVER_START_REQ( get_message ) { req->internal = filter->internal; req->flags = flags; @@ -2716,9 +2736,10 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) req->get_first = first; req->get_last = last; req->hw_id = hw_id; - req->wake_mask = filter->mask & (QS_SENDMESSAGE | QS_SMRESULT); + req->wake_mask = wake_mask; req->changed_mask = filter->mask; wine_server_set_reply( req, buffer, buffer_size ); + thread_info->last_getmsg_time = NtGetTickCount(); if (!(res = wine_server_call( req ))) { size = wine_server_reply_size( reply ); diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index e9d407efb14..a6e61e1751b 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -108,6 +108,7 @@ struct user_thread_info { struct ntuser_thread_info client_info; /* Data shared with client */ HANDLE server_queue; /* Handle to server-side queue */ + DWORD last_getmsg_time; /* Get/PeekMessage last request time */ WORD message_count; /* Get/PeekMessage loop counter */ WORD hook_call_depth; /* Number of recursively called hook procs */ WORD hook_unicode; /* Is current hook unicode? */ diff --git a/server/queue.c b/server/queue.c index 4f58b795b7e..2a7e1d07082 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3154,7 +3154,7 @@ DECL_HANDLER(get_message) struct msg_queue *queue = get_current_queue(); user_handle_t get_win = get_user_full_handle( req->get_win ); const queue_shm_t *queue_shm; - unsigned int filter = req->flags >> 16; + unsigned int filter; if (get_win && get_win != 1 && get_win != -1 && !get_user_object( get_win, USER_WINDOW )) { @@ -3177,7 +3177,6 @@ DECL_HANDLER(get_message) } queue->last_get_msg = current_time; - if (!filter) filter = QS_ALLINPUT; /* first check for sent messages */ if ((ptr = list_head( &queue->msg_list[SEND_MESSAGE] ))) @@ -3187,6 +3186,9 @@ DECL_HANDLER(get_message) return; } + /* use the same logic as in win32u/message.c peek_message */ + if (!(filter = req->flags >> 16)) filter = QS_ALLINPUT; + /* clear changed bits so we can wait on them if we don't find a message */ SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5970
From: Rémi Bernon <rbernon(a)codeweavers.com> As we don't need to make a server request to check for the queue status anymore, we can stop relying on ProcessEvent return status, and only call it often enough, before and after checking for messages. We keep a throttle here though, as now that PeekMessage returns very quickly when no message is found, calling it in a tight loop triggers a large number of calls into the drivers instead. --- dlls/win32u/message.c | 37 +++++++++++++----------------------- dlls/win32u/ntuser_private.h | 2 +- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 1cb6e3189e6..62c789a1532 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -3022,19 +3022,14 @@ static HANDLE get_server_queue_handle(void) } /* check for driver events if we detect that the app is not properly consuming messages */ -static inline void check_for_driver_events( UINT msg ) +static inline void check_for_driver_events(void) { - if (get_user_thread_info()->message_count > 200) + if (get_user_thread_info()->last_driver_time != NtGetTickCount()) { flush_window_surfaces( FALSE ); user_driver->pProcessEvents( QS_ALLINPUT ); + get_user_thread_info()->last_driver_time = NtGetTickCount(); } - else if (msg == WM_TIMER || msg == WM_SYSTIMER) - { - /* driver events should have priority over timers, so make sure we'll check for them soon */ - get_user_thread_info()->message_count += 100; - } - else get_user_thread_info()->message_count++; } /* helper for kernel32->ntdll timeout format conversion */ @@ -3059,8 +3054,8 @@ static DWORD wait_message( DWORD count, const HANDLE *handles, DWORD timeout, DW lock = *(DWORD *)ret_ptr; } - if (user_driver->pProcessEvents( mask )) ret = count ? count - 1 : 0; - else if (count) + if (user_driver->pProcessEvents( mask )) ret = count - 1; + else { ret = NtWaitForMultipleObjects( count, handles, !(flags & MWMO_WAITALL), !!(flags & MWMO_ALERTABLE), get_nt_timeout( &time, timeout )); @@ -3071,10 +3066,9 @@ static DWORD wait_message( DWORD count, const HANDLE *handles, DWORD timeout, DW ret = WAIT_FAILED; } } - else ret = WAIT_TIMEOUT; if (ret == WAIT_TIMEOUT && !count && !timeout) NtYieldExecution(); - if ((mask & QS_INPUT) == QS_INPUT) get_user_thread_info()->message_count = 0; + if (ret == count - 1) get_user_thread_info()->last_driver_time = NtGetTickCount(); if (enable_thunk_lock) KeUserModeCallback( NtUserThunkLock, &lock, sizeof(lock), &ret_ptr, &ret_len ); @@ -3231,20 +3225,15 @@ BOOL WINAPI NtUserPeekMessage( MSG *msg_out, HWND hwnd, UINT first, UINT last, U int ret; user_check_not_lock(); - check_for_driver_events( 0 ); + check_for_driver_events(); - ret = peek_message( &msg, &filter ); - if (ret < 0) return FALSE; - - if (!ret) + if ((ret = peek_message( &msg, &filter )) <= 0) { - flush_window_surfaces( TRUE ); - ret = wait_message( 0, NULL, 0, QS_ALLINPUT, 0 ); - /* if we received driver events, check again for a pending message */ - if (ret == WAIT_TIMEOUT || peek_message( &msg, &filter ) <= 0) return FALSE; + if (!ret) flush_window_surfaces( TRUE ); + return FALSE; } - check_for_driver_events( msg.message ); + check_for_driver_events(); /* copy back our internal safe copy of message data to msg_out. * msg_out is a variable from the *program*, so it can't be used @@ -3270,7 +3259,7 @@ BOOL WINAPI NtUserGetMessage( MSG *msg, HWND hwnd, UINT first, UINT last ) int ret; user_check_not_lock(); - check_for_driver_events( 0 ); + check_for_driver_events(); if (first || last) { @@ -3291,7 +3280,7 @@ BOOL WINAPI NtUserGetMessage( MSG *msg, HWND hwnd, UINT first, UINT last ) } if (ret < 0) return -1; - check_for_driver_events( msg->message ); + check_for_driver_events(); return msg->message != WM_QUIT; } diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index a6e61e1751b..a361e1d73a5 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -109,7 +109,7 @@ struct user_thread_info struct ntuser_thread_info client_info; /* Data shared with client */ HANDLE server_queue; /* Handle to server-side queue */ DWORD last_getmsg_time; /* Get/PeekMessage last request time */ - WORD message_count; /* Get/PeekMessage loop counter */ + DWORD last_driver_time; /* Get/PeekMessage driver event time */ WORD hook_call_depth; /* Number of recursively called hook procs */ WORD hook_unicode; /* Is current hook unicode? */ HHOOK hook; /* Current hook */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5970
LGTM. Tested it in games, everything is fine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5970#note_74925
I can reproduce the mshtml tests failure, it's probably related to the changes, I'll have a look. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5970#note_74995
On Wed Jul 3 10:11:07 2024 +0000, Rémi Bernon wrote:
I can reproduce the mshtml tests failure, it's probably related to the changes, I'll have a look. Actually I can also reproduce it with current tip, so doesn't seem related to these changes. It happens for me pretty consistently when running the test in the background inside docker, not so much when running it directly.
The ddraw failures also don't seem directly related, and instead look like some race conditions with pending ConfigureNotify events. I'll send a separate MR to work around that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5970#note_75009
Alexandre Julliard (@julliard) commented about dlls/win32u/message.c:
if (res) { - free( buffer ); + if (buffer != buffer_init) free( buffer ); if (res == STATUS_PENDING) { + NtYieldExecution();
I think this should be up to the caller. It's not useful to yield if we are going to wait anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5970#note_75186
participants (3)
-
Alexandre Julliard (＠julliard) -
Grigory Vasilyev (＠h0tc0d3)
-
Rémi Bernon