include/tools/time.hxx | 20 ++++++++ tools/Library_tl.mk | 1 tools/source/datetime/ttime.cxx | 66 +++++++++++++++++++--------- vcl/inc/win/saldata.hxx | 4 - vcl/inc/win/saltimer.h | 31 +++++++++++-- vcl/win/app/salinst.cxx | 42 +++++++++++------- vcl/win/app/saltimer.cxx | 93 +++++++++++++++++++--------------------- 7 files changed, 164 insertions(+), 93 deletions(-)
New commits: commit d8ab2834ca4fa571e45e837eac061b94b7385c1c Author: Jan-Marek Glogowski <[email protected]> AuthorDate: Thu Aug 24 13:41:37 2017 +0200 Commit: Thorsten Behrens <[email protected]> CommitDate: Wed Jan 16 19:31:28 2019 +0100 tdf#111994 WIN workaround PostMessage delays Fixes the "Multiple timers in queue" assertion by effectively removing it. When debugging it became obvious, that PostMessage returns, even if the message was not yet added to the message queue. The assert happens, because we start the timer in the Scheduler before Invoke(), so it fires, if we block in Invoke(), and then reset the timer after Invoke, if there were changes to the Task list. In this case it fires during Invoke(), the message is added. We restart the timer, first by stopping it (we wait in DeleteTimerQueueTimer, to be sure the timer function has either finished or was not run). And the try to remove the message with PeekMessageW, which doesn't remove the posted message. Then the timer is restarted, and when the event is processed, we end up with an additional timer event, which was asserted. As a fix this adds a (microsecond) timestamp to the timer message, which is validated in the WinProc function. So if we stop the timer too fast, the event is ignored based on the timestamp. And while at it, the patch moves timer related variables from SalData into WinSalTimer. Change-Id: Ib840a421e8bd040d40f39473e1d44491e5b332bd Reviewed-on: https://gerrit.libreoffice.org/42575 Tested-by: Jenkins <[email protected]> Reviewed-by: Jan-Marek Glogowski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/66446 Reviewed-by: Thorsten Behrens <[email protected]> Tested-by: Thorsten Behrens <[email protected]> diff --git a/vcl/inc/win/saldata.hxx b/vcl/inc/win/saldata.hxx index 7b6440cda9e0..0a1943e8d8a9 100644 --- a/vcl/inc/win/saldata.hxx +++ b/vcl/inc/win/saldata.hxx @@ -80,8 +80,6 @@ public: long* mpDitherDiff; // Dither mapping table BYTE* mpDitherLow; // Dither mapping table BYTE* mpDitherHigh; // Dither mapping table - HANDLE mnTimerId; ///< Windows timer id - bool mbOnIdleRunScheduler; ///< Run yield until the scheduler processed the idle HHOOK mhSalObjMsgHook; // hook to get interesting msg for SalObject HWND mhWantLeaveMsg; // window handle, that want a MOUSELEAVE message AutoTimer* mpMouseLeaveTimer; // Timer for MouseLeave Test @@ -169,8 +167,6 @@ void ImplSalAcquireYieldMutex( sal_uLong nCount ); LRESULT CALLBACK SalFrameWndProcW( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lParam ); -void EmitTimerCallback(); - void SalTestMouseLeave(); void ImplWriteLastError(DWORD lastError, const char *szApiCall); diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h index f9390583b7b2..9107dd1a0b19 100644 --- a/vcl/inc/win/saltimer.h +++ b/vcl/inc/win/saltimer.h @@ -24,16 +24,39 @@ class WinSalTimer : public SalTimer { + HANDLE m_nTimerId; ///< Windows timer id + sal_uInt32 m_nTimerStartTicks; ///< system ticks at timer start % SAL_MAX_UINT32 + bool m_bPollForMessage; ///< Run yield until a message is caught (most likely the 0ms timer) + public: - WinSalTimer() {} - virtual ~WinSalTimer(); + WinSalTimer(); + virtual ~WinSalTimer() override; virtual void Start(sal_uIntPtr nMS) override; virtual void Stop() override; + + inline bool IsValidWPARAM( WPARAM wParam ) const; + + inline bool PollForMessage() const; + + // The Impl functions are just public to be called from the static + // SalComWndProc on main thread redirect! Otherwise they would be private. + // They must be called from the main application thread only! + + void ImplStart( sal_uIntPtr nMS ); + void ImplStop(); + void ImplEmitTimerCallback(); }; -void ImplSalStartTimer( sal_uIntPtr nMS ); -void ImplSalStopTimer(); +inline bool WinSalTimer::IsValidWPARAM( WPARAM aWPARAM ) const +{ + return aWPARAM == m_nTimerStartTicks; +} + +inline bool WinSalTimer::PollForMessage() const +{ + return m_bPollForMessage; +} #endif diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index 8e37cc6af2c2..707df7cc9aa5 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -336,8 +336,6 @@ SalData::SalData() mpDitherDiff = 0; // Dither mapping table mpDitherLow = 0; // Dither mapping table mpDitherHigh = 0; // Dither mapping table - mnTimerId = 0; // windows timer id - mbOnIdleRunScheduler = false; // if yield is idle, run the scheduler mhSalObjMsgHook = 0; // hook to get interesting msg for SalObject mhWantLeaveMsg = 0; // window handle, that want a MOUSELEAVE message mpMouseLeaveTimer = 0; // Timer for MouseLeave Test @@ -349,6 +347,7 @@ SalData::SalData() mpHDCCache = 0; // Cache for three DC's mh50Bmp = 0; // 50% Bitmap mh50Brush = 0; // 50% Brush + int i; for(i=0; i<MAX_STOCKPEN; i++) { @@ -581,7 +580,8 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) { MSG aMsg; bool bWasMsg = false, bOneEvent = false; - SalData *const pSalData = GetSalData(); + ImplSVData *const pSVData = ImplGetSVData(); + WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer ); int nMaxEvents = bHandleAllCurrentEvents ? 100 : 1; do @@ -597,20 +597,22 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) // busy loop to catch the 0ms timeout // We don't need to busy loop, if we wait anyway. // Even if we didn't process the event directly, report it. - if ( pSalData->mbOnIdleRunScheduler && !bWait ) + if ( pTimer && pTimer->PollForMessage() && !bWait ) { SwitchToThread(); nMaxEvents++; bOneEvent = true; bWasMsg = true; } - } while( --nMaxEvents && bOneEvent ); + } + while( --nMaxEvents && bOneEvent ); - if ( bWait && ! bWasMsg ) + // Also check that we don't wait when application already has quit + if ( bWait && !bWasMsg && !pSVData->maAppData.mbAppQuit ) { if ( GetMessageW( &aMsg, 0, 0, 0 ) ) { - // Ignore the scheduler wakeup message + bWasMsg = true; TranslateMessage( &aMsg ); ImplSalDispatchMessage( &aMsg ); } @@ -675,17 +677,17 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i break; case SAL_MSG_STARTTIMER: { - sal_uLong nTime = GetTickCount(); - if ( nTime < (sal_uLong) lParam ) - nTime = (sal_uLong) lParam - nTime; + sal_uInt64 nTime = tools::Time::GetSystemTicks(); + if ( nTime < (sal_uInt64) lParam ) + nTime = (sal_uInt64) lParam - nTime; else nTime = 0; - ImplSalStartTimer( nTime ); + static_cast<WinSalTimer*>(ImplGetSVData()->maSchedCtx.mpSalTimer)->ImplStart( nTime ); rDef = FALSE; break; } case SAL_MSG_STOPTIMER: - ImplSalStopTimer(); + static_cast<WinSalTimer*>(ImplGetSVData()->maSchedCtx.mpSalTimer)->ImplStop(); break; case SAL_MSG_CREATEFRAME: nRet = (LRESULT)ImplSalCreateFrame( GetSalData()->mpFirstInstance, (HWND)lParam, (SalFrameStyleFlags)wParam ); @@ -732,14 +734,22 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i rDef = FALSE; break; case SAL_MSG_TIMER_CALLBACK: + { + WinSalTimer *const pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer ); + assert( pTimer != nullptr ); MSG aMsg; + bool bValidMSG = pTimer->IsValidWPARAM( wParam ); // PM_QS_POSTMESSAGE is needed, so we don't process the SendMessage from DoYield! - while ( PeekMessageW(&aMsg, nullptr, SAL_MSG_TIMER_CALLBACK, + while ( PeekMessageW(&aMsg, GetSalData()->mpFirstInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK, SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) ) - assert( "Multiple timer messages in queue" ); - GetSalData()->mbOnIdleRunScheduler = false; - EmitTimerCallback(); + { + assert( !bValidMSG && "Unexpected non-last valid message" ); + bValidMSG = pTimer->IsValidWPARAM( aMsg.wParam ); + } + if ( bValidMSG ) + pTimer->ImplEmitTimerCallback(); break; + } } return nRet; diff --git a/vcl/win/app/saltimer.cxx b/vcl/win/app/saltimer.cxx index 5dc1bfca1165..a57fddf30426 100644 --- a/vcl/win/app/saltimer.cxx +++ b/vcl/win/app/saltimer.cxx @@ -35,31 +35,29 @@ static void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired); // deletion of timer (which is extremely likely, given that // INVALID_HANDLE_VALUE waits for the callback to run on the main thread), // this must run on the main thread too -void ImplSalStopTimer() +void WinSalTimer::ImplStop() { SalData *const pSalData = GetSalData(); - assert( !pSalData->mpFirstInstance || pSalData->mnAppThreadId == GetCurrentThreadId() ); + const WinSalInstance *pInst = pSalData->mpFirstInstance; + assert( !pInst || pSalData->mnAppThreadId == GetCurrentThreadId() ); - HANDLE hTimer = pSalData->mnTimerId; - if (hTimer) - { - pSalData->mnTimerId = 0; - DeleteTimerQueueTimer(NULL, hTimer, INVALID_HANDLE_VALUE); - } + const HANDLE hTimer = m_nTimerId; + if ( nullptr == hTimer ) + return; + + m_nTimerId = nullptr; + m_nTimerStartTicks = 0; + DeleteTimerQueueTimer( nullptr, hTimer, INVALID_HANDLE_VALUE ); + m_bPollForMessage = false; - // remove all pending SAL_MSG_TIMER_CALLBACK messages - // we always have to do this, since ImplSalStartTimer with 0ms just queues - // a new SAL_MSG_TIMER_CALLBACK message + // remove as many pending SAL_MSG_TIMER_CALLBACK messages as possible // PM_QS_POSTMESSAGE is needed, so we don't process the SendMessage from DoYield! MSG aMsg; - int nMsgCount = 0; - while ( PeekMessageW(&aMsg, nullptr, SAL_MSG_TIMER_CALLBACK, - SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) ) - nMsgCount++; - assert( nMsgCount <= 1 ); + while ( PeekMessageW(&aMsg, pInst->mhComWnd, SAL_MSG_TIMER_CALLBACK, + SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) ); } -void ImplSalStartTimer( sal_uLong nMS ) +void WinSalTimer::ImplStart( sal_uLong nMS ) { SalData* pSalData = GetSalData(); assert( !pSalData->mpFirstInstance || pSalData->mnAppThreadId == GetCurrentThreadId() ); @@ -69,17 +67,26 @@ void ImplSalStartTimer( sal_uLong nMS ) nMS = SAL_MAX_UINT32; // cannot change a one-shot timer, so delete it and create a new one - ImplSalStopTimer(); + ImplStop(); - // keep the scheduler running, if a 0ms timer / Idle is scheduled - pSalData->mbOnIdleRunScheduler = ( 0 == nMS ); + // keep the yield running, if a 0ms Idle is scheduled + m_bPollForMessage = ( 0 == nMS ); + m_nTimerStartTicks = tools::Time::GetMonotonicTicks() % SAL_MAX_UINT32; // probably WT_EXECUTEONLYONCE is not needed, but it enforces Period // to be 0 and should not hurt; also see // https://www.microsoft.com/msj/0499/pooling/pooling.aspx - CreateTimerQueueTimer(&pSalData->mnTimerId, nullptr, SalTimerProc, nullptr, + CreateTimerQueueTimer(&m_nTimerId, nullptr, SalTimerProc, + (void*) m_nTimerStartTicks, nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE); } +WinSalTimer::WinSalTimer() + : m_nTimerId( nullptr ) + , m_nTimerStartTicks( 0 ) + , m_bPollForMessage( false ) +{ +} + WinSalTimer::~WinSalTimer() { Stop(); @@ -87,28 +94,28 @@ WinSalTimer::~WinSalTimer() void WinSalTimer::Start( sal_uLong nMS ) { - SalData* pSalData = GetSalData(); - if ( pSalData->mpFirstInstance && pSalData->mnAppThreadId != GetCurrentThreadId() ) + WinSalInstance *pInst = GetSalData()->mpFirstInstance; + if ( pInst && !pInst->IsMainThread() ) { - BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, - SAL_MSG_STARTTIMER, 0, (LPARAM)GetTickCount() + nMS); + BOOL const ret = PostMessageW(pInst->mhComWnd, + SAL_MSG_STARTTIMER, 0, (LPARAM) tools::Time::GetSystemTicks() + nMS); SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!"); } else - ImplSalStartTimer( nMS ); + ImplStart( nMS ); } void WinSalTimer::Stop() { - SalData* pSalData = GetSalData(); - if ( pSalData->mpFirstInstance && pSalData->mnAppThreadId != GetCurrentThreadId() ) + WinSalInstance *pInst = GetSalData()->mpFirstInstance; + if ( pInst && !pInst->IsMainThread() ) { - BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, + BOOL const ret = PostMessageW(pInst->mhComWnd, SAL_MSG_STOPTIMER, 0, 0); SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!"); } else - ImplSalStopTimer(); + ImplStop(); } /** This gets invoked from a Timer Queue thread. @@ -116,7 +123,7 @@ void WinSalTimer::Stop() Don't acquire the SolarMutex to avoid deadlocks, just wake up the main thread at better resolution than 10ms. */ -static void CALLBACK SalTimerProc(PVOID, BOOLEAN) +static void CALLBACK SalTimerProc(PVOID data, BOOLEAN) { #if defined ( __MINGW32__ ) && !defined ( _WIN64 ) jmp_buf jmpbuf; @@ -128,17 +135,14 @@ static void CALLBACK SalTimerProc(PVOID, BOOLEAN) __try { #endif - - SalData* pSalData = GetSalData(); - // always post message when the timer fires, we will remove the ones // that happened during execution of the callback later directly from // the message queue - BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, - SAL_MSG_TIMER_CALLBACK, 0, 0); + BOOL const ret = PostMessageW(GetSalData()->mpFirstInstance->mhComWnd, + SAL_MSG_TIMER_CALLBACK, (WPARAM) data, 0); #if OSL_DEBUG_LEVEL > 0 if (0 == ret) // SEH prevents using SAL_WARN here? - fputs("ERROR: PostMessage() failed!", stderr); + fputs("ERROR: PostMessage() failed!\n", stderr); #endif #if defined ( __MINGW32__ ) && !defined ( _WIN64 ) @@ -150,24 +154,17 @@ static void CALLBACK SalTimerProc(PVOID, BOOLEAN) { } #endif -} -/** Called in the main thread. +} -We assured that by posting the message from the SalTimeProc only, the real -call then happens when the main thread gets SAL_MSG_TIMER_CALLBACK. -*/ -void EmitTimerCallback() +void WinSalTimer::ImplEmitTimerCallback() { // Test for MouseLeave SalTestMouseLeave(); - ImplSVData *pSVData = ImplGetSVData(); - if ( ! pSVData->maSchedCtx.mpSalTimer ) - return; - + m_bPollForMessage = false; ImplSalYieldMutexAcquireWithWait(); - pSVData->maSchedCtx.mpSalTimer->CallCallback(); + CallCallback(); ImplSalYieldMutexRelease(); } commit 8ae649d197da3e15d6531cf31682b4e761be87fa Author: Jan-Marek Glogowski <[email protected]> AuthorDate: Tue Aug 15 23:22:36 2017 +0200 Commit: Thorsten Behrens <[email protected]> CommitDate: Wed Jan 16 19:31:15 2019 +0100 Partial 'Add tools::Time::GetMonotonicTicks (us)' bubli's notes: reduced to introducing the func in tools folder. canvas and opencl left untouched original commit message: This moves a combination of tools::Time::GetSystemTicks(), canvas ElapsedTime::getSystemTime() and the opencl timing implementation into tools::Time::GetMonotonicTicks() as a monotonic microsecond time source. Change-Id: I5c9263540b8af55b2eeca6126e288129427f6e8e Reviewed-on: https://gerrit.libreoffice.org/41991 Tested-by: Jenkins <[email protected]> Reviewed-by: Stephan Bergmann <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/66445 Reviewed-by: Thorsten Behrens <[email protected]> Tested-by: Thorsten Behrens <[email protected]> diff --git a/include/tools/time.hxx b/include/tools/time.hxx index 12f1e909c8ec..f12425831e88 100644 --- a/include/tools/time.hxx +++ b/include/tools/time.hxx @@ -121,9 +121,27 @@ public: static Time GetUTCOffset(); - /// Elapsed time since epoch in milliseconds + /** + * Elapsed time in milliseconds (1e-3) since some unspecified starting point + * + * Convenience function, which just calls GetMonotonicTicks() / 1000. + */ static sal_uInt64 GetSystemTicks(); + /** + * Elapsed time in microseconds (1e-6) since some unspecified starting point + * + * Uses the high-precision, monotonic time sources provided by the OS, if + * available. Don't try to relate it to the system time, and also it's long + * time accuracy is not the best. + * + * Currently used to measure the runtime of OpenCL shaders and to set a + * message creation timestamp to allow filtering of invalid timer messages. + * + * @return current system ticks in microseconds (1e-6s) + */ + static sal_uInt64 GetMonotonicTicks(); + tools::Time& operator =( const tools::Time& rTime ); Time operator -() const { return Time( -nTime ); } diff --git a/tools/Library_tl.mk b/tools/Library_tl.mk index 2916ad3becf4..591b2bbbee76 100644 --- a/tools/Library_tl.mk +++ b/tools/Library_tl.mk @@ -108,6 +108,7 @@ $(eval $(call gb_Library_use_system_win32_libs,tl,\ ole32 \ shell32 \ uuid \ + winmm \ )) endif diff --git a/tools/source/datetime/ttime.cxx b/tools/source/datetime/ttime.cxx index 91c8ea09223a..362ff470a738 100644 --- a/tools/source/datetime/ttime.cxx +++ b/tools/source/datetime/ttime.cxx @@ -23,6 +23,7 @@ #if defined(_WIN32) #include <windows.h> +#include <mmsystem.h> #elif defined UNX #include <unistd.h> #include <limits.h> @@ -34,6 +35,7 @@ #ifdef __MACH__ #include <mach/clock.h> #include <mach/mach.h> +#include <mach/mach_time.h> #endif #include <sal/log.hxx> @@ -411,30 +413,54 @@ Time tools::Time::GetUTCOffset() sal_uInt64 tools::Time::GetSystemTicks() { -#if defined(_WIN32) - static LARGE_INTEGER nTicksPerSecond; - static bool bTicksPerSecondInitialized = false; - if (!bTicksPerSecondInitialized) - { - QueryPerformanceFrequency(&nTicksPerSecond); - bTicksPerSecondInitialized = true; - } + return tools::Time::GetMonotonicTicks() / 1000; +} - LARGE_INTEGER nPerformanceCount; - QueryPerformanceCounter(&nPerformanceCount); +#ifdef _WIN32 +static LARGE_INTEGER initPerformanceFrequency() +{ + LARGE_INTEGER nTicksPerSecond = { 0 }; + if (!QueryPerformanceFrequency(&nTicksPerSecond)) + nTicksPerSecond.QuadPart = 0; + return nTicksPerSecond; +} +#endif - return static_cast<sal_uInt64>( - (nPerformanceCount.QuadPart*1000)/nTicksPerSecond.QuadPart); -#else - timeval tv; - int n = gettimeofday (&tv, nullptr); - if (n == -1) { - int e = errno; - SAL_WARN("tools.datetime", "gettimeofday failed: " << e); +sal_uInt64 tools::Time::GetMonotonicTicks() +{ +#ifdef _WIN32 + static const LARGE_INTEGER nTicksPerSecond = initPerformanceFrequency(); + if (nTicksPerSecond.QuadPart > 0) + { + LARGE_INTEGER nPerformanceCount; + QueryPerformanceCounter(&nPerformanceCount); + return static_cast<sal_uInt64>( + ( nPerformanceCount.QuadPart * 1000 * 1000 ) / nTicksPerSecond.QuadPart ); } - return static_cast<sal_uInt64>(tv.tv_sec) * 1000 - + (static_cast<sal_uInt64>(tv.tv_usec) + 500) / 1000; + else + { + return static_cast<sal_uInt64>( timeGetTime() * 1000 ); + } +#else + sal_uInt64 nMicroSeconds; +#ifdef __MACH__ + static mach_timebase_info_data_t info = { 0, 0 }; + if ( 0 == info.numer ) + mach_timebase_info( &info ); + nMicroSeconds = mach_absolute_time() * (double) (info.numer / info.denom) / 1000; +#else +#if defined(USE_CLOCK_GETTIME) + struct timespec currentTime; + clock_gettime( CLOCK_MONOTONIC, ¤tTime ); + nMicroSeconds = currentTime.tv_sec * 1000 * 1000 + currentTime.tv_nsec / 1000; +#else + struct timeval currentTime; + gettimeofday( ¤tTime, nullptr ); + nMicroSeconds = currentTime.tv_sec * 1000 * 1000 + currentTime.tv_usec; #endif +#endif // __MACH__ + return nMicroSeconds; +#endif // _WIN32 } } /* namespace tools */ _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
