[Libreoffice-commits] core.git: vcl/inc vcl/README.scheduler vcl/win
Jan-Marek Glogowski
glogow at fbihome.de
Fri Oct 13 14:48:53 UTC 2017
vcl/README.scheduler | 10 +++++----
vcl/inc/win/saltimer.h | 22 ++++++++-------------
vcl/win/app/salinst.cxx | 41 ++++++++++++++++-----------------------
vcl/win/app/saltimer.cxx | 49 ++++++++++++++++++++++++-----------------------
4 files changed, 58 insertions(+), 64 deletions(-)
New commits:
commit fb4e7be5d4eac6d7c429c215e72de45ea28d86cd
Author: Jan-Marek Glogowski <glogow at fbihome.de>
Date: Thu Oct 12 16:00:42 2017 +0200
WIN another system loop integration attempt
This time we skip the intention to handle our Scheduler
completely via the system event loop. Instead we basically
guarantee to process a Scheduler event during each DoYield,
if one is available. This way we won't block system events
when busy in our event loop.
Change-Id: I37094e61cbf928733151d9cc3299cdac76b3a5cd
Reviewed-on: https://gerrit.libreoffice.org/43349
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index 8c5e64ba74c5..a052dd420c74 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -180,10 +180,12 @@ based on the function used to generate them. Even if WM_TIMER messages should
have the lowest priority, a manually posted WM_TIMER is processed with the
priority of a PostMessage message.
-Therefore the current solution always starts a (threaded) timer, even for the
-instant Idles, and syncs to this timer message in the main dispatch loop, to
-process the messages in the correct sequence. Using SwitchToThread(), this
-kind of polling seem to work reasonably well.
+So we're giving up on processing all our Scheduler events as a message in the
+system message loop. Instead we just indicate a 0ms timer message by setting
+the m_bDirectTimeout in the timer object. This timer is always processed, if
+the system message wasn't already our timer. As a result we can also skip the
+polling. All this is one more reason to drop the single message processing
+in favour of always processing all pending (system) events.
An additional workaround is implemented for the delayed queuing of posted
messages, where PeekMessage in WinSalTimer::Stop() won't be able remove the
diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h
index 5ad6a1718f19..37976bbfaf8b 100644
--- a/vcl/inc/win/saltimer.h
+++ b/vcl/inc/win/saltimer.h
@@ -26,15 +26,18 @@ class WinSalTimer final : public SalTimer, protected VersionedEvent
{
// for access to Impl* functions
friend LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, int& rDef );
- // for access to m_bPollForMessage
+ // for access to GetNextVersionedEvent
friend void CALLBACK SalTimerProc( PVOID data, BOOLEAN );
+ // for access to ImplHandleElapsedTimer
+ friend bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents );
HANDLE m_nTimerId; ///< Windows timer id
- bool m_bPollForMessage; ///< Run yield until a message is caught (most likely the 0ms timer)
+ bool m_bDirectTimeout; ///< timeout can be processed directly
void ImplStart( sal_uIntPtr nMS );
void ImplStop();
- void ImplEmitTimerCallback();
+ void ImplHandleTimerEvent( WPARAM aWPARAM );
+ void ImplHandleElapsedTimer();
public:
WinSalTimer();
@@ -43,19 +46,12 @@ public:
virtual void Start(sal_uIntPtr nMS) override;
virtual void Stop() override;
- inline bool IsValidWPARAM( WPARAM wParam ) const;
-
- inline bool PollForMessage() const;
+ inline bool IsDirectTimeout() const;
};
-inline bool WinSalTimer::IsValidWPARAM( WPARAM aWPARAM ) const
-{
- return IsValidEventVersion( static_cast<sal_Int32>( aWPARAM ) );
-}
-
-inline bool WinSalTimer::PollForMessage() const
+inline bool WinSalTimer::IsDirectTimeout() const
{
- return m_bPollForMessage;
+ return m_bDirectTimeout;
}
#endif
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index a0a323b2d037..38db3d3bb17c 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -472,11 +472,11 @@ static void ImplSalDispatchMessage( MSG* pMsg )
ImplSalPostDispatchMsg( pMsg, lResult );
}
-static bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
+bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
{
static sal_uInt32 nLastTicks = 0;
MSG aMsg;
- bool bWasMsg = false, bOneEvent = false;
+ bool bWasMsg = false, bOneEvent = false, bWasTimeoutMsg = false;
ImplSVData *const pSVData = ImplGetSVData();
WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer );
@@ -491,6 +491,8 @@ static bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
if ( bOneEvent )
{
bWasMsg = true;
+ if ( !bWasTimeoutMsg )
+ bWasTimeoutMsg = (SAL_MSG_TIMER_CALLBACK == aMsg.message);
TranslateMessage( &aMsg );
ImplSalDispatchMessage( &aMsg );
if ( bHandleAllCurrentEvents
@@ -499,23 +501,26 @@ static bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
bHadNewerEvent = true;
bOneEvent = !bHadNewerEvent;
}
- // busy loop to catch a message, eventually the 0ms timer.
- // we don't need to loop, if we wait anyway.
- if ( !bWait && !bWasMsg && pTimer && pTimer->PollForMessage() )
- {
- SwitchToThread();
- continue;
- }
+
if ( !(bHandleAllCurrentEvents && bOneEvent) )
break;
}
while( true );
+ // 0ms timeouts are handled out-of-bounds to prevent busy-locking the
+ // event loop with timeout massages.
+ // We ensure we never handle more then one timeout per call.
+ // This way we'll always process a normal system message.
+ if ( !bWasTimeoutMsg && pTimer && pTimer->IsDirectTimeout() )
+ {
+ pTimer->ImplHandleElapsedTimer();
+ bWasMsg = true;
+ }
+
if ( bHandleAllCurrentEvents )
nLastTicks = nCurTicks;
- // Also check that we don't wait when application already has quit
- if ( bWait && !bWasMsg && !pSVData->maAppData.mbAppQuit )
+ if ( bWait && !bWasMsg )
{
if ( GetMessageW( &aMsg, nullptr, 0, 0 ) )
{
@@ -540,8 +545,6 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
SolarMutexReleaser aReleaser;
if ( !IsMainThread() )
{
- // If you change the SendMessageW function, you might need to update
- // the PeekMessage( ... PM_QS_POSTMESSAGE) calls!
bDidWork = SendMessageW( mhComWnd, SAL_MSG_THREADYIELD,
(WPARAM) false, (LPARAM) bHandleAllCurrentEvents );
if ( !bDidWork && bWait )
@@ -659,17 +662,7 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i
{
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, pInst->mhComWnd, SAL_MSG_TIMER_CALLBACK,
- SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) )
- {
- assert( !bValidMSG && "Unexpected non-last valid message" );
- bValidMSG = pTimer->IsValidWPARAM( aMsg.wParam );
- }
- if ( bValidMSG )
- pTimer->ImplEmitTimerCallback();
+ pTimer->ImplHandleTimerEvent( wParam );
break;
}
}
diff --git a/vcl/win/app/saltimer.cxx b/vcl/win/app/saltimer.cxx
index 93b93fbb832f..fe22d53db8c8 100644
--- a/vcl/win/app/saltimer.cxx
+++ b/vcl/win/app/saltimer.cxx
@@ -46,16 +46,11 @@ void WinSalTimer::ImplStop()
return;
m_nTimerId = nullptr;
+ m_bDirectTimeout = false;
DeleteTimerQueueTimer( nullptr, hTimer, INVALID_HANDLE_VALUE );
- // Keep both after DeleteTimerQueueTimer, because they are set in SalTimerProc
+ // Keep InvalidateEvent after DeleteTimerQueueTimer, because the event id
+ // is set in SalTimerProc, which DeleteTimerQueueTimer will finish or cancel.
InvalidateEvent();
- m_bPollForMessage = false;
-
- // 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;
- while ( PeekMessageW(&aMsg, pInst->mhComWnd, SAL_MSG_TIMER_CALLBACK,
- SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) );
}
void WinSalTimer::ImplStart( sal_uLong nMS )
@@ -70,20 +65,21 @@ void WinSalTimer::ImplStart( sal_uLong nMS )
// cannot change a one-shot timer, so delete it and create a new one
ImplStop();
- // keep the yield running, if a 0ms Idle is scheduled
- m_bPollForMessage = ( 0 == nMS );
+ // directly indicate an elapsed timer
+ m_bDirectTimeout = ( 0 == nMS );
// 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(&m_nTimerId, nullptr, SalTimerProc,
- reinterpret_cast<void*>(
- sal_IntPtr(GetNextEventVersion())),
- nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
+ if ( !m_bDirectTimeout )
+ CreateTimerQueueTimer(&m_nTimerId, nullptr, SalTimerProc, this,
+ nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
+ // We don't need any wakeup message, as this code can just run in the
+ // main thread!
}
WinSalTimer::WinSalTimer()
: m_nTimerId( nullptr )
- , m_bPollForMessage( false )
+ , m_bDirectTimeout( false )
{
}
@@ -126,12 +122,10 @@ void CALLBACK SalTimerProc(PVOID data, BOOLEAN)
{
__try
{
- // 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(GetSalData()->mpInstance->mhComWnd,
- SAL_MSG_TIMER_CALLBACK,
- reinterpret_cast<WPARAM>(data), 0);
+ WinSalTimer *pTimer = reinterpret_cast<WinSalTimer*>( data );
+ BOOL const ret = PostMessageW(
+ GetSalData()->mpInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK,
+ static_cast<WPARAM>(pTimer->GetNextEventVersion()), 0 );
#if OSL_DEBUG_LEVEL > 0
if (0 == ret) // SEH prevents using SAL_WARN here?
fputs("ERROR: PostMessage() failed!\n", stderr);
@@ -142,15 +136,24 @@ void CALLBACK SalTimerProc(PVOID data, BOOLEAN)
}
}
-void WinSalTimer::ImplEmitTimerCallback()
+void WinSalTimer::ImplHandleElapsedTimer()
{
// Test for MouseLeave
SalTestMouseLeave();
- m_bPollForMessage = false;
+ m_bDirectTimeout = false;
ImplSalYieldMutexAcquireWithWait();
CallCallback();
ImplSalYieldMutexRelease();
}
+void WinSalTimer::ImplHandleTimerEvent( const WPARAM aWPARAM )
+{
+ assert( aWPARAM <= SAL_MAX_INT32 );
+ if ( !IsValidEventVersion( static_cast<sal_Int32>( aWPARAM ) ) )
+ return;
+
+ ImplHandleElapsedTimer();
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list