flush CallbackFlushHandler queue via PostUserEvent instead of Idle+Timer

Looking at when Invoke was called from Idle (or from auxiliary Timeout
if there was no chance to run the Idle within 100ms) and when duplicate
merging and modification of the queue is done before getting dispatched
and cleared; Then the pattern is that duplicates are collected and
merged during the current block of events to be processed, and
dispatching the flush via PostUserEvent instead gives the same results.

During startup, scheduling via PostUserEvent drops the need to have the
aux timeout, and while the number of messages seen in the first queue
flush (now via PostUserEvent instead of 100ms Timer) remain the same as
before, subsequent queue flushes can get processed earlier while
smaller, though typically at the same time as before once the document
is fully loaded.

Change-Id: I64c6bbc3dea9fb3a512c2a9521303a8f143d4a89
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167064
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167307
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>

1 files changed, 28 insertions, 34 deletions

diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index 30d2d824e5c6..e093bf7ff4f2 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1502,32 +1502,14 @@ static OUString getGenerator()
 
 extern "C" {
 
-CallbackFlushHandler::TimeoutIdle::TimeoutIdle( CallbackFlushHandler* handler )
-    : Timer( "lokit timer callback" )
-    , mHandler( handler )
-{
-    // A second timer with higher priority, it'll ensure we flush in reasonable time if we get too busy
-    // to get POST_PAINT priority processing. Otherwise it could take a long time to flush.
-    SetPriority(TaskPriority::DEFAULT);
-    SetTimeout( 100 ); // 100 ms
-}
-
-void CallbackFlushHandler::TimeoutIdle::Invoke()
-{
-    mHandler->Invoke();
-}
-
 // One of these is created per view to handle events cf. doc_registerCallback
 CallbackFlushHandler::CallbackFlushHandler(LibreOfficeKitDocument* pDocument, LibreOfficeKitCallback pCallback, void* pData)
-    : Idle( "lokit idle callback" ),
-      m_pDocument(pDocument),
+    : m_pDocument(pDocument),
       m_pCallback(pCallback),
+      m_pFlushEvent(nullptr),
       m_pData(pData),
-      m_nDisableCallbacks(0),
-      m_TimeoutIdle( this )
+      m_nDisableCallbacks(0)
 {
-    SetPriority(TaskPriority::POST_PAINT);
-
     // Add the states that are safe to skip duplicates on, even when
     // not consequent (i.e. do no emit them if unchanged from last).
     m_states.emplace(LOK_CALLBACK_TEXT_SELECTION, "NIL"_ostr);
@@ -1546,9 +1528,18 @@ CallbackFlushHandler::CallbackFlushHandler(LibreOfficeKitDocument* pDocument, Li
     m_states.emplace(LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE, "NIL"_ostr);
 }
 
+void CallbackFlushHandler::stop()
+{
+    if (m_pFlushEvent)
+    {
+        Application::RemoveUserEvent(m_pFlushEvent);
+        m_pFlushEvent = nullptr;
+    }
+}
+
 CallbackFlushHandler::~CallbackFlushHandler()
 {
-    Stop();
+    stop();
 }
 
 CallbackFlushHandler::queue_type2::iterator CallbackFlushHandler::toQueue2(CallbackFlushHandler::queue_type1::iterator pos)
@@ -1570,7 +1561,7 @@ void CallbackFlushHandler::setUpdatedType( int nType, bool value )
         m_updatedTypes.resize( nType + 1 ); // new are default-constructed, i.e. false
     m_updatedTypes[ nType ] = value;
     if(value)
-        startTimer();
+        scheduleFlush();
 }
 
 void CallbackFlushHandler::resetUpdatedType( int nType )
@@ -1586,7 +1577,7 @@ void CallbackFlushHandler::setUpdatedTypePerViewId( int nType, int nViewId, int
         types.resize( nType + 1 ); // new are default-constructed, i.e. 'set' is false
     types[ nType ] = PerViewIdData{ value, nSourceViewId };
     if(value)
-        startTimer();
+        scheduleFlush();
 }
 
 void CallbackFlushHandler::resetUpdatedTypePerViewId( int nType, int nViewId )
@@ -1663,7 +1654,7 @@ void CallbackFlushHandler::dumpState(rtl::OStringBuffer &rState)
 void CallbackFlushHandler::libreOfficeKitViewAddPendingInvalidateTiles()
 {
     // Invoke() will call flushPendingLOKInvalidateTiles(), so just make sure the timer is active.
-    startTimer();
+    scheduleFlush();
 }
 
 void CallbackFlushHandler::queue(const int type, const OString& data)
@@ -1953,7 +1944,7 @@ void CallbackFlushHandler::queue(const int type, CallbackData& aCallbackData)
 #endif
 
     lock.unlock();
-    startTimer();
+    scheduleFlush();
 }
 
 bool CallbackFlushHandler::processInvalidateTilesEvent(int type, CallbackData& aCallbackData)
@@ -2331,7 +2322,7 @@ void CallbackFlushHandler::enqueueUpdatedType( int type, const SfxViewShell* vie
         << "] to have " << m_queue1.size() << " entries.");
 }
 
-void CallbackFlushHandler::Invoke()
+void CallbackFlushHandler::invoke()
 {
     comphelper::ProfileZone aZone("CallbackFlushHandler::Invoke");
 
@@ -2439,16 +2430,19 @@ void CallbackFlushHandler::Invoke()
 
     m_queue1.clear();
     m_queue2.clear();
-    Stop();
-    m_TimeoutIdle.Stop();
+    stop();
 }
 
-void CallbackFlushHandler::startTimer()
+void CallbackFlushHandler::scheduleFlush()
 {
-    if (!IsActive())
-        Start();
-    if (!m_TimeoutIdle.IsActive())
-        m_TimeoutIdle.Start();
+    if (!m_pFlushEvent)
+        m_pFlushEvent = Application::PostUserEvent(LINK(this, CallbackFlushHandler, FlushQueue));
+}
+
+IMPL_LINK_NOARG(CallbackFlushHandler, FlushQueue, void*, void)
+{
+    m_pFlushEvent = nullptr;
+    invoke();
 }
 
 bool CallbackFlushHandler::removeAll(int type)