[PATCH 1/2] winmm: Use condition variable to sleep in timer thread.

[Jacek Caban]

We have fast condition variable implementation now, so performance 
should not be a problem.

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
---
  dlls/winmm/time.c | 81 ++++++++---------------------------------------
  1 file changed, 14 insertions(+), 67 deletions(-)

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=54363

Your paranoid android.


=== debian9 (build log) ===

Task: WineTest did not produce the wow32 report

[Andrew Eikum]

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>

On Tue, Jul 02, 2019 at 08:00:31PM +0200, Jacek Caban wrote:
> We have fast condition variable implementation now, so performance should
> not be a problem.
>
> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
> ---
>  dlls/winmm/time.c | 81 ++++++++---------------------------------------
>  1 file changed, 14 insertions(+), 67 deletions(-)

> diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c
> index e055f6bb96..22c89852a2 100644
> --- a/dlls/winmm/time.c
> +++ b/dlls/winmm/time.c
> @@ -20,24 +20,9 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
>   */
>
> -#include "config.h"
> -#include "wine/port.h"
> -
>  #include <stdarg.h>
>  #include <errno.h>
>  #include <time.h>
> -#ifdef HAVE_SYS_TIME_H
> -# include <sys/time.h>
> -#endif
> -#ifdef HAVE_UNISTD_H
> -# include <unistd.h>
> -#endif
> -#ifdef HAVE_POLL_H
> -#include <poll.h>
> -#endif
> -#ifdef HAVE_SYS_POLL_H
> -#include <sys/poll.h>
> -#endif
>
> #include "windef.h"
>  #include "winbase.h"
> @@ -74,7 +59,7 @@ static CRITICAL_SECTION TIME_cbcrst = { &critsect_debug, -1, 0, 0, 0, 0 };
>
> static    HANDLE                TIME_hMMTimer;
>  static    BOOL                  TIME_TimeToDie = TRUE;
> -static    int                   TIME_fdWake[2] = { -1, -1 };
> +static    CONDITION_VARIABLE    TIME_cv;
>
> /* link timer at the appropriate spot in the list */
>  static inline void link_timer( WINE_TIMERENTRY *timer )
> @@ -116,8 +101,6 @@ static inline void link_timer( WINE_TIMERENTRY *timer )
>  #define MMSYSTIME_MININTERVAL (1)
>  #define MMSYSTIME_MAXINTERVAL (65535)
>
> -#ifdef HAVE_POLL
> -
>  /**************************************************************************
>   *           TIME_MMSysTimeCallback
>   */
> @@ -195,12 +178,8 @@ static int TIME_MMSysTimeCallback(void)
>   */
>  static DWORD CALLBACK TIME_MMSysTimeThread(LPVOID arg)
>  {
> -    int sleep_time, ret;
> -    char readme[16];
> -    struct pollfd pfd;
> -
> -    pfd.fd = TIME_fdWake[0];
> -    pfd.events = POLLIN;
> +    int sleep_time;
> +    BOOL ret;
>
> TRACE("Starting main winmm thread\n");
>
> @@ -214,20 +193,12 @@ static DWORD CALLBACK TIME_MMSysTimeThread(LPVOID arg)
>          if (sleep_time == 0)
>              continue;
>
> -        LeaveCriticalSection(&WINMM_cs);
> -        ret = poll(&pfd, 1, sleep_time);
> -        EnterCriticalSection(&WINMM_cs);
> -
> -        if (ret < 0)
> +        ret = SleepConditionVariableCS(&TIME_cv, &WINMM_cs, sleep_time);
> +        if (!ret && GetLastError() != ERROR_TIMEOUT)
>          {
> -            if (errno != EINTR && errno != EAGAIN)
> -            {
> -                ERR("Unexpected error in poll: %s(%d)\n", strerror(errno), errno);
> -                break;
> -            }
> +            ERR("Unexpected error in poll: %s(%d)\n", strerror(errno), errno);
> +            break;
>           }
> -
> -        while (ret > 0) ret = read(TIME_fdWake[0], readme, sizeof(readme));
>      }
>      CloseHandle(TIME_hMMTimer);
>      TIME_hMMTimer = NULL;
> @@ -242,35 +213,15 @@ static DWORD CALLBACK TIME_MMSysTimeThread(LPVOID arg)
>   */
>  static void TIME_MMTimeStart(void)
>  {
> +    HMODULE mod;
>      TIME_TimeToDie = 0;
> +    if (TIME_hMMTimer) return;
>
> -    if (TIME_fdWake[0] < 0) {
> -        if (pipe(TIME_fdWake) < 0) {
> -            TIME_fdWake[0] = TIME_fdWake[1] = -1;
> -            ERR("Cannot create pipe: %s\n", strerror(errno));
> -        } else {
> -            fcntl(TIME_fdWake[0], F_SETFL, O_NONBLOCK);
> -            fcntl(TIME_fdWake[1], F_SETFL, O_NONBLOCK);
> -        }
> -    }
> -
> -    if (!TIME_hMMTimer) {
> -        HMODULE mod;
> -        GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, (LPCWSTR)TIME_MMSysTimeThread, &mod);
> -        TIME_hMMTimer = CreateThread(NULL, 0, TIME_MMSysTimeThread, mod, 0, NULL);
> -        SetThreadPriority(TIME_hMMTimer, THREAD_PRIORITY_TIME_CRITICAL);
> -    }
> +    GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, (LPCWSTR)TIME_MMSysTimeThread, &mod);
> +    TIME_hMMTimer = CreateThread(NULL, 0, TIME_MMSysTimeThread, mod, 0, NULL);
> +    SetThreadPriority(TIME_hMMTimer, THREAD_PRIORITY_TIME_CRITICAL);
>  }
>
> -#else  /* HAVE_POLL */
> -
> -static void TIME_MMTimeStart(void)
> -{
> -    FIXME( "not starting system thread\n" );
> -}
> -
> -#endif  /* HAVE_POLL */
> -
>  /**************************************************************************
>   * 				TIME_MMTimeStop
>   */
> @@ -282,8 +233,6 @@ void	TIME_MMTimeStop(void)
>              ERR("Timer still active?!\n");
>              CloseHandle(TIME_hMMTimer);
>          }
> -        close(TIME_fdWake[0]);
> -        close(TIME_fdWake[1]);
>          DeleteCriticalSection(&TIME_cbcrst);
>      }
>  }
> @@ -312,7 +261,6 @@ MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc,
>      WORD 		wNewID = 0;
>      LPWINE_TIMERENTRY	lpNewTimer;
>      LPWINE_TIMERENTRY	lpTimer;
> -    const char c = 'c';
>
> TRACE("(%u, %u, %p, %08lX, %04X);\n", wDelay, wResol, lpFunc, dwUser, wFlags);
>
> @@ -346,7 +294,7 @@ MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc,
>      LeaveCriticalSection(&WINMM_cs);
>
> /* Wake the service thread in case there is work to be done */
> -    write(TIME_fdWake[1], &c, sizeof(c));
> +    WakeConditionVariable(&TIME_cv);
>
> TRACE("=> %u\n", wNewID + 1);
>
> @@ -373,9 +321,8 @@ MMRESULT WINAPI timeKillEvent(UINT wID)
>   }
>      }
>      if (list_empty(&timer_list)) {
> -        char c = 'q';
>          TIME_TimeToDie = 1;
> -        write(TIME_fdWake[1], &c, sizeof(c));
> +        WakeConditionVariable(&TIME_cv);
>      }
>      LeaveCriticalSection(&WINMM_cs);

>