From fea27b13484f50f3beae834122c648dd8ce7248f Mon Sep 17 00:00:00 2001 From: Jeremy White Date: Mon, 18 Oct 2004 21:44:32 +0000 Subject: [PATCH] Revise winmm/time.c to fix timer resolution at 1 ms. This then allows a much more efficient implementation of timer events and timeGetTime, and callers that used sub 10 ms resolution will now get correct results. --- dlls/winmm/mmsystem.c | 5 +- dlls/winmm/time.c | 291 ++++++++++++++++++++++++++++-------------- dlls/winmm/winemm.h | 3 +- 3 files changed, 195 insertions(+), 104 deletions(-) diff --git a/dlls/winmm/mmsystem.c b/dlls/winmm/mmsystem.c index 18ed860cb38..563c79ee11f 100644 --- a/dlls/winmm/mmsystem.c +++ b/dlls/winmm/mmsystem.c @@ -2524,12 +2524,9 @@ void MMSYSTEM_MMTIME16to32(LPMMTIME mmt32, const MMTIME16* mmt16) */ MMRESULT16 WINAPI timeGetSystemTime16(LPMMTIME16 lpTime, UINT16 wSize) { - TRACE("(%p, %u);\n", lpTime, wSize); - if (wSize >= sizeof(*lpTime)) { lpTime->wType = TIME_MS; - TIME_MMTimeStart(); - lpTime->u.ms = WINMM_SysTimeMS; + lpTime->u.ms = GetTickCount(); TRACE("=> %lu\n", lpTime->u.ms); } diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c index a6148836be2..7ecc1b83375 100644 --- a/dlls/winmm/time.c +++ b/dlls/winmm/time.c @@ -45,23 +45,44 @@ WINE_DEFAULT_DEBUG_CHANNEL(mmtime); static HANDLE TIME_hMMTimer; static LPWINE_TIMERENTRY TIME_TimersList; static HANDLE TIME_hKillEvent; -DWORD WINMM_SysTimeMS; +static HANDLE TIME_hWakeEvent; +static BOOL TIME_TimeToDie = TRUE; /* - * FIXME - * We're using "1" as the mininum resolution to the timer, - * as Windows 95 does, according to the docs. Maybe it should - * depend on the computers resources! + * Some observations on the behavior of winmm on Windows. + * First, the call to timeBeginPeriod(xx) can never be used + * to raise the timer resolution, only lower it. + * + * Second, a brief survey of a variety of Win 2k and Win X + * machines showed that a 'standard' (aka default) timer + * resolution was 1 ms (Win9x is documented as being 1). However, one + * machine had a standard timer resolution of 10 ms. + * + * Further, if we set our default resolution to 1, + * the implementation of timeGetTime becomes GetTickCount(), + * and we can optimize the code to reduce overhead. + * + * Additionally, a survey of Event behaviors shows that + * if we request a Periodic event every 50 ms, then Windows + * makes sure to trigger that event 20 times in the next + * second. If delays prevent that from happening on exact + * schedule, Windows will trigger the events as close + * to the original schedule as is possible, and will eventually + * bring the event triggers back onto a schedule that is + * consistent with what would have happened if there were + * no delays. + * + * Jeremy White, October 2004 */ #define MMSYSTIME_MININTERVAL (1) #define MMSYSTIME_MAXINTERVAL (65535) -#define MMSYSTIME_STDINTERVAL (10) /* reasonable value? */ static void TIME_TriggerCallBack(LPWINE_TIMERENTRY lpTimer) { - TRACE("before CallBack => lpFunc=%p wTimerID=%04X dwUser=%08lX !\n", - lpTimer->lpFunc, lpTimer->wTimerID, lpTimer->dwUser); + TRACE("%04lx:CallBack => lpFunc=%p wTimerID=%04X dwUser=%08lX dwTriggerTime %ld(delta %ld)\n", + GetCurrentThreadId(), lpTimer->lpFunc, lpTimer->wTimerID, lpTimer->dwUser, + lpTimer->dwTriggerTime, GetTickCount() - lpTimer->dwTriggerTime); /* - TimeProc callback that is called here is something strange, under Windows 3.1x it is called * during interrupt time, is allowed to execute very limited number of API calls (like @@ -87,83 +108,116 @@ static void TIME_TriggerCallBack(LPWINE_TIMERENTRY lpTimer) lpTimer->wFlags, lpTimer->lpFunc); break; } - TRACE("after CallBack !\n"); } /************************************************************************** * TIME_MMSysTimeCallback */ -static void CALLBACK TIME_MMSysTimeCallback(LPWINE_MM_IDATA iData) +static DWORD CALLBACK TIME_MMSysTimeCallback(LPWINE_MM_IDATA iData) { static int nSizeLpTimers; static LPWINE_TIMERENTRY lpTimers; LPWINE_TIMERENTRY timer, *ptimer, *next_ptimer; - DWORD delta = GetTickCount() - WINMM_SysTimeMS; int idx; + DWORD cur_time; + DWORD delta_time; + DWORD ret_time = INFINITE; + DWORD adjust_time; - TRACE("Time delta: %ld\n", delta); - while (delta >= MMSYSTIME_MININTERVAL) { - delta -= MMSYSTIME_MININTERVAL; - WINMM_SysTimeMS += MMSYSTIME_MININTERVAL; + /* optimize for the most frequent case - no events */ + if (! TIME_TimersList) + return(ret_time); - /* since timeSetEvent() and timeKillEvent() can be called - * from 16 bit code, there are cases where win16 lock is - * locked upon entering timeSetEvent(), and then the mm timer - * critical section is locked. This function cannot call the - * timer callback with the crit sect locked (because callback - * may need to acquire Win16 lock, thus providing a deadlock - * situation). - * To cope with that, we just copy the WINE_TIMERENTRY struct - * that need to trigger the callback, and call it without the - * mm timer crit sect locked. - * the hKillTimeEvent is used to mark the section where we - * handle the callbacks so we can do synchronous kills. - * EPP 99/07/13, updated 04/01/10 - */ - idx = 0; + /* since timeSetEvent() and timeKillEvent() can be called + * from 16 bit code, there are cases where win16 lock is + * locked upon entering timeSetEvent(), and then the mm timer + * critical section is locked. This function cannot call the + * timer callback with the crit sect locked (because callback + * may need to acquire Win16 lock, thus providing a deadlock + * situation). + * To cope with that, we just copy the WINE_TIMERENTRY struct + * that need to trigger the callback, and call it without the + * mm timer crit sect locked. + * the hKillTimeEvent is used to mark the section where we + * handle the callbacks so we can do synchronous kills. + * EPP 99/07/13, updated 04/01/10 + */ + idx = 0; + cur_time = GetTickCount(); - EnterCriticalSection(&iData->cs); - for (ptimer = &TIME_TimersList; *ptimer != NULL; ) { - timer = *ptimer; - next_ptimer = &timer->lpNext; - if (timer->uCurTime < MMSYSTIME_MININTERVAL) { - /* since lpTimer->wDelay is >= MININTERVAL, wCurTime value - * shall be correct (>= 0) - */ - timer->uCurTime += timer->wDelay - MMSYSTIME_MININTERVAL; - if (timer->lpFunc) { - if (idx == nSizeLpTimers) { - if (lpTimers) - lpTimers = (LPWINE_TIMERENTRY) - HeapReAlloc(GetProcessHeap(), 0, lpTimers, - ++nSizeLpTimers * sizeof(WINE_TIMERENTRY)); - else - lpTimers = (LPWINE_TIMERENTRY) - HeapAlloc(GetProcessHeap(), 0, - ++nSizeLpTimers * sizeof(WINE_TIMERENTRY)); - } - lpTimers[idx++] = *timer; - } - /* TIME_ONESHOT is defined as 0 */ - if (!(timer->wFlags & TIME_PERIODIC)) - { - /* unlink timer from timers list */ - *ptimer = *next_ptimer; - HeapFree(GetProcessHeap(), 0, timer); + EnterCriticalSection(&iData->cs); + for (ptimer = &TIME_TimersList; *ptimer != NULL; ) { + timer = *ptimer; + next_ptimer = &timer->lpNext; + if (cur_time >= timer->dwTriggerTime) + { + if (timer->lpFunc) { + if (idx == nSizeLpTimers) { + if (lpTimers) + lpTimers = (LPWINE_TIMERENTRY) + HeapReAlloc(GetProcessHeap(), 0, lpTimers, + ++nSizeLpTimers * sizeof(WINE_TIMERENTRY)); + else + lpTimers = (LPWINE_TIMERENTRY) + HeapAlloc(GetProcessHeap(), 0, + ++nSizeLpTimers * sizeof(WINE_TIMERENTRY)); } - } else { - timer->uCurTime -= MMSYSTIME_MININTERVAL; - } - ptimer = next_ptimer; - } - if (TIME_hKillEvent) ResetEvent(TIME_hKillEvent); - LeaveCriticalSection(&iData->cs); + lpTimers[idx++] = *timer; - while (idx > 0) TIME_TriggerCallBack(&lpTimers[--idx]); - if (TIME_hKillEvent) SetEvent(TIME_hKillEvent); + } + + /* Update the time after we make the copy to preserve + the original trigger time */ + timer->dwTriggerTime += timer->wDelay; + + /* TIME_ONESHOT is defined as 0 */ + if (!(timer->wFlags & TIME_PERIODIC)) + { + /* unlink timer from timers list */ + *ptimer = *next_ptimer; + HeapFree(GetProcessHeap(), 0, timer); + + /* We don't need to trigger oneshots again */ + delta_time = INFINITE; + } + else + { + /* Compute when this event needs this function + to be called again */ + if (timer->dwTriggerTime <= cur_time) + delta_time = 0; + else + delta_time = timer->dwTriggerTime - cur_time; + } + } + else + delta_time = timer->dwTriggerTime - cur_time; + + /* Determine when we need to return to this function */ + ret_time = min(ret_time, delta_time); + + ptimer = next_ptimer; } + if (TIME_hKillEvent) ResetEvent(TIME_hKillEvent); + LeaveCriticalSection(&iData->cs); + + while (idx > 0) TIME_TriggerCallBack(&lpTimers[--idx]); + if (TIME_hKillEvent) SetEvent(TIME_hKillEvent); + + /* Finally, adjust the recommended wait time downward + by the amount of time the processing routines + actually took */ + adjust_time = GetTickCount() - cur_time; + if (adjust_time > ret_time) + ret_time = 0; + else + ret_time -= adjust_time; + + /* We return the amount of time our caller should sleep + before needing to check in on us again */ + return(ret_time); } /************************************************************************** @@ -172,18 +226,43 @@ static LPWINE_TIMERENTRY lpTimers; static DWORD CALLBACK TIME_MMSysTimeThread(LPVOID arg) { LPWINE_MM_IDATA iData = (LPWINE_MM_IDATA)arg; - volatile HANDLE *pActive = (volatile HANDLE *)&TIME_hMMTimer; - DWORD last_time, cur_time; + DWORD sleep_time; + DWORD rc; - usleep(MMSYSTIME_STDINTERVAL * 1000); - last_time = GetTickCount(); - while (*pActive) { - TIME_MMSysTimeCallback(iData); - cur_time = GetTickCount(); - while (last_time < cur_time) - last_time += MMSYSTIME_STDINTERVAL; - usleep((last_time - cur_time) * 1000); + TRACE("Starting main winmm thread\n"); + + /* FIXME: As an optimization, we could have + this thread die when there are no more requests + pending, and then get recreated on the first + new event; it's not clear if that would be worth + it or not. */ + + while (! TIME_TimeToDie) + { + sleep_time = TIME_MMSysTimeCallback(iData); + + if (sleep_time == 0) + { + /* This Sleep is controversial; it was added to make + Wine able to replicate a high speed (e.g. 1 ms) + timer event where the called event routine chews + a lot of CPU. This is required because of the + bias some Linux kernel versions have against threads that + chew a lot of the CPU; this Sleep(0) yields enough + in that spin case doesn't trigger the bias. + Further, it should do no harm, but an fyi. */ + Sleep(0); + continue; + } + + rc = WaitForSingleObject(TIME_hWakeEvent, sleep_time); + if (rc != WAIT_TIMEOUT && rc != WAIT_OBJECT_0) + { + FIXME("Unexpected error %ld(%ld) in timer thread\n", rc, GetLastError()); + break; + } } + TRACE("Exiting main winmm thread\n"); return 0; } @@ -192,13 +271,10 @@ static DWORD CALLBACK TIME_MMSysTimeThread(LPVOID arg) */ void TIME_MMTimeStart(void) { - /* one could think it's possible to stop the service thread activity when no more - * mm timers are active, but this would require to keep mmSysTimeMS up-to-date - * without being incremented within the service thread callback. - */ if (!TIME_hMMTimer) { - WINMM_SysTimeMS = GetTickCount(); TIME_TimersList = NULL; + TIME_hWakeEvent = CreateEventW(NULL, FALSE, FALSE, NULL); + TIME_TimeToDie = FALSE; TIME_hMMTimer = CreateThread(NULL, 0, TIME_MMSysTimeThread, WINMM_IData, 0, NULL); } } @@ -208,12 +284,18 @@ void TIME_MMTimeStart(void) */ void TIME_MMTimeStop(void) { - /* FIXME: in the worst case, we're going to wait 65 seconds here :-( */ if (TIME_hMMTimer) { - HANDLE hMMTimer = TIME_hMMTimer; + + TIME_TimeToDie = TRUE; + SetEvent(TIME_hWakeEvent); + + /* FIXME: in the worst case, we're going to wait 65 seconds here :-( */ + WaitForSingleObject(TIME_hMMTimer, INFINITE); + + CloseHandle(TIME_hMMTimer); + CloseHandle(TIME_hWakeEvent); TIME_hMMTimer = 0; - WaitForSingleObject(hMMTimer, INFINITE); - CloseHandle(hMMTimer); + TIME_TimersList = NULL; } } @@ -222,14 +304,11 @@ void TIME_MMTimeStop(void) */ MMRESULT WINAPI timeGetSystemTime(LPMMTIME lpTime, UINT wSize) { - TRACE("(%p, %u);\n", lpTime, wSize); if (wSize >= sizeof(*lpTime)) { - TIME_MMTimeStart(); lpTime->wType = TIME_MS; - lpTime->u.ms = WINMM_SysTimeMS; + lpTime->u.ms = GetTickCount(); - TRACE("=> %lu\n", lpTime->u.ms); } return 0; @@ -256,8 +335,11 @@ WORD TIME_SetEventInternal(UINT wDelay, UINT wResol, TIME_MMTimeStart(); - lpNewTimer->uCurTime = wDelay; lpNewTimer->wDelay = wDelay; + lpNewTimer->dwTriggerTime = GetTickCount() + wDelay; + + /* FIXME - wResol is not respected, although it is not clear + that we could change our precision meaningfully */ lpNewTimer->wResol = wResol; lpNewTimer->lpFunc = lpFunc; lpNewTimer->dwUser = dwUser; @@ -278,6 +360,9 @@ WORD TIME_SetEventInternal(UINT wDelay, UINT wResol, LeaveCriticalSection(&WINMM_IData->cs); + /* Wake the service thread in case there is work to be done */ + SetEvent(TIME_hWakeEvent); + TRACE("=> %u\n", wNewID + 1); return wNewID + 1; @@ -332,7 +417,7 @@ MMRESULT WINAPI timeKillEvent(UINT wID) */ MMRESULT WINAPI timeGetDevCaps(LPTIMECAPS lpCaps, UINT wSize) { - TRACE("(%p, %u) !\n", lpCaps, wSize); + TRACE("(%p, %u)\n", lpCaps, wSize); lpCaps->wPeriodMin = MMSYSTIME_MININTERVAL; lpCaps->wPeriodMax = MMSYSTIME_MAXINTERVAL; @@ -344,10 +429,14 @@ MMRESULT WINAPI timeGetDevCaps(LPTIMECAPS lpCaps, UINT wSize) */ MMRESULT WINAPI timeBeginPeriod(UINT wPeriod) { - TRACE("(%u) !\n", wPeriod); - if (wPeriod < MMSYSTIME_MININTERVAL || wPeriod > MMSYSTIME_MAXINTERVAL) return TIMERR_NOCANDO; + + if (wPeriod > MMSYSTIME_MININTERVAL) + { + FIXME("Stub; we set our timer resolution at minimum\n"); + } + return 0; } @@ -356,10 +445,13 @@ MMRESULT WINAPI timeBeginPeriod(UINT wPeriod) */ MMRESULT WINAPI timeEndPeriod(UINT wPeriod) { - TRACE("(%u) !\n", wPeriod); - if (wPeriod < MMSYSTIME_MININTERVAL || wPeriod > MMSYSTIME_MAXINTERVAL) return TIMERR_NOCANDO; + + if (wPeriod > MMSYSTIME_MININTERVAL) + { + FIXME("Stub; we set our timer resolution at minimum\n"); + } return 0; } @@ -369,12 +461,15 @@ MMRESULT WINAPI timeEndPeriod(UINT wPeriod) */ DWORD WINAPI timeGetTime(void) { +#if defined(COMMENTOUTPRIORTODELETING) DWORD count; + /* FIXME: releasing the win16 lock here is a temporary hack (I hope) * that lets mciavi.drv run correctly */ if (pFnReleaseThunkLock) pFnReleaseThunkLock(&count); - TIME_MMTimeStart(); if (pFnRestoreThunkLock) pFnRestoreThunkLock(count); - return WINMM_SysTimeMS; +#endif + + return GetTickCount(); } diff --git a/dlls/winmm/winemm.h b/dlls/winmm/winemm.h index cdd93522771..59db93d281a 100644 --- a/dlls/winmm/winemm.h +++ b/dlls/winmm/winemm.h @@ -162,7 +162,7 @@ typedef struct tagWINE_TIMERENTRY { DWORD dwUser; UINT16 wFlags; UINT16 wTimerID; - UINT uCurTime; + DWORD dwTriggerTime; struct tagWINE_TIMERENTRY* lpNext; } WINE_TIMERENTRY, *LPWINE_TIMERENTRY; @@ -285,7 +285,6 @@ void TIME_MMTimeStop(void); /* Global variables */ extern LPWINE_MM_IDATA WINMM_IData; -extern DWORD WINMM_SysTimeMS; /* pointers to 16 bit functions (if sibling MMSYSTEM.DLL is loaded * NULL otherwise