[PATCH v2 0/3] MR5970: win32u: Use the thread message queue shared memory in peek_message.
-- v2: win32u: Simplify the logic for driver messages polling. win32u: Use the thread message queue shared memory in peek_message. win32u: Allocate heap in peek_message only when necessary.
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/message.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 2f5ffc3a538..0a7102fece2 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,11 +2738,8 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter )
if (res) { - free( buffer ); - if (res == STATUS_PENDING) - { - return 0; - } + if (buffer != buffer_init) free( buffer ); + if (res == STATUS_PENDING) return 0; if (res != STATUS_BUFFER_OVERFLOW) { RtlSetLastWin32Error( RtlNtStatusToDosError(res) ); @@ -2766,6 +2762,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 +2846,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 +2874,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 +2889,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 +2938,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; }
From: Rémi Bernon rbernon@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 0a7102fece2..20674a11b1e 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 ) {
From: Rémi Bernon rbernon@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 | 41 +++++++++++++++--------------------- dlls/win32u/ntuser_private.h | 2 +- 2 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 20674a11b1e..14597d4f51a 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -3018,19 +3018,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 */ @@ -3055,8 +3050,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 )); @@ -3067,10 +3062,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 ); @@ -3227,20 +3221,19 @@ BOOL WINAPI NtUserPeekMessage( MSG *msg_out, HWND hwnd, UINT first, UINT last, U int ret;
user_check_not_lock(); - check_for_driver_events( 0 ); - - ret = peek_message( &msg, &filter ); - if (ret < 0) return FALSE; + check_for_driver_events();
- 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 ); + NtYieldExecution(); + } + 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 @@ -3266,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) { @@ -3287,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 */
On Fri Jul 5 18:00:48 2024 +0000, Rémi Bernon wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5970/diffs?diff_id=120753&start_sha=d7bfba3249951b16b970d2d7bbd87622b0e9a578#bddee5ea9d993e9a0987591fc4e612261c3a87a3_2765_2763)
Sure, I've moved the yield to NtUserPeekMessage instead and merged with the change that reduces the pressure on the driver ProcessEvent calls.
-
Rémi Bernon -
Rémi Bernon (＠rbernon)