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@libreoffice.org>
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

4 files changed, 58 insertions, 64 deletions

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: */