Re: [PATCH 4/5] user32: Only recheck monitor cache once per second.

[Zhiyi Zhang]

On 7/24/20 5:14 PM, Matteo Bruni wrote:
> Signed-off-by: Matteo Bruni
>
> dlls/user32/sysparams.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/dlls/user32/sysparams.c b/dlls/user32/sysparams.c
> index d4462e589e8a..52a32d5e1c59 100644
> --- a/dlls/user32/sysparams.c
> +++ b/dlls/user32/sysparams.c
> @@ -313,6 +313,7 @@ static const WCHAR GUID_DEVINTERFACE_MONITOR[] = {'#','{','e','6','f','0','7','b
>  /* Cached monitor information */
>  static MONITORINFOEXW *monitors;
>  static UINT monitor_count;
> +static DWORD last_cache_check_time;
>  static FILETIME last_query_monitors_time;
>  static CRITICAL_SECTION monitors_section;
>  static CRITICAL_SECTION_DEBUG monitors_critsect_debug =
> @@ -3952,13 +3953,19 @@ static BOOL update_monitor_cache(void)
>      DWORD i = 0, j;
>      DWORD type;
> -
>
> if (monitor_count && GetTickCount() - last_cache_check_time < 1000)
> -
>    return TRUE;
> -
>
> /* Update monitor cache from SetupAPI if it's outdated */
>    if (!video_key && RegOpenKeyW( HKEY_LOCAL_MACHINE, VIDEO_KEY, &video_key ))
>        return FALSE;
>    if (RegQueryInfoKeyW( video_key, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, &filetime ))
>        return FALSE;
>    if (CompareFileTime( &filetime, &last_query_monitors_time ) < 1)
> -
>
> {
> -
>    last_cache_check_time = GetTickCount();
>    return TRUE;
> -
>
> }
>
> mutex = get_display_device_init_mutex();
> EnterCriticalSection( &monitors_section );
>
> @@ -4020,6 +4027,7 @@ static BOOL update_monitor_cache(void)
>      }
>  last_query_monitors_time = filetime;
> - last_cache_check_time = GetTickCount();
> ret = TRUE;
>
> fail:
>      SetupDiDestroyDeviceInfoList( devinfo );

I think this will cause a bunch of test failures if monitor changes are not reflected immediately.
The real overhead here is the wine server calls, which would need something like a wine server
using shared memory to reduce latency.

Another way that might be acceptable is resetting last_cache_check_time in ChangeDisplaySettingsExW()
when the current display mode is changed. So for at least the current process, the changes are reflected immediately.
The downside is that other processes might have a 1 second delay, but that might be okay because before
this monitor cache is implemented, other processes are not aware of the monitor changes happened in another
process at all, and it seemed to be working fine at the time.

Thanks,
Zhiyi