optimize removing from the LOK flush queue

All the lambdas check for event type, so it makes sense to first
separately check the type and only then possibly call the lambda.
Especially since 3b3e4ee97af23f21 separated the types for better
searching.

Change-Id: I144c88f5319ac2141336e1aa3c4ffd7b38265af9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122673
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 114 insertions, 93 deletions

diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx
index e6b4f00a5427..2693e67eb718 100644
--- a/desktop/inc/lib/init.hxx
+++ b/desktop/inc/lib/init.hxx
@@ -128,9 +128,12 @@ namespace desktop {
         typedef std::vector<CallbackData> queue_type2;
 
     private:
+        bool removeAll(int type);
+        bool removeAll(int type, const std::function<bool (const CallbackData&)>& rTestFunc);
         bool removeAll(const std::function<bool (int, const CallbackData&)>& rTestFunc);
         bool processInvalidateTilesEvent(int type, CallbackData& aCallbackData);
         bool processWindowEvent(int type, CallbackData& aCallbackData);
+        queue_type2::iterator toQueue2(queue_type1::iterator);
         queue_type2::reverse_iterator toQueue2(queue_type1::reverse_iterator);
 
         /** we frequently want to scan the queue, and mostly when we do so, we only care about the element type
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index 26ba3016b99c..267d54492d14 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1426,6 +1426,12 @@ void CallbackFlushHandler::callback(const int type, const char* payload, void* d
     }
 }
 
+CallbackFlushHandler::queue_type2::iterator CallbackFlushHandler::toQueue2(CallbackFlushHandler::queue_type1::iterator pos)
+{
+    int delta = std::distance(m_queue1.begin(), pos);
+    return m_queue2.begin() + delta;
+}
+
 CallbackFlushHandler::queue_type2::reverse_iterator CallbackFlushHandler::toQueue2(CallbackFlushHandler::queue_type1::reverse_iterator pos)
 {
     int delta = std::distance(m_queue1.rbegin(), pos);
@@ -1555,8 +1561,7 @@ void CallbackFlushHandler::queue(const int type, const char* data)
             case LOK_CALLBACK_GRAPHIC_SELECTION:
             case LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR:
             case LOK_CALLBACK_INVALIDATE_TILES:
-                if (removeAll(
-                        [type](int elemType, const CallbackData&) { return (elemType == type); }))
+                if (removeAll(type))
                     SAL_INFO("lok", "Removed dups of [" << type << "]: [" << payload << "].");
                 break;
         }
@@ -1579,8 +1584,7 @@ void CallbackFlushHandler::queue(const int type, const char* data)
             case LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE:
             case LOK_CALLBACK_RULER_UPDATE:
             {
-                if (removeAll(
-                        [type](int elemType, const CallbackData&) { return (elemType == type); }))
+                if (removeAll(type))
                     SAL_INFO("lok", "Removed dups of [" << type << "]: [" << payload << "].");
             }
             break;
@@ -1603,9 +1607,8 @@ void CallbackFlushHandler::queue(const int type, const char* data)
                     payload.find("\"hyperlink\":\"\"") == std::string::npos &&
                     payload.find("\"hyperlink\": {}") == std::string::npos;
                 const int nViewId = lcl_getViewId(payload);
-                removeAll(
-                    [type, nViewId, hyperLinkException] (int elemType, const CallbackData& elemData) {
-                        return (elemType == type && nViewId == lcl_getViewId(elemData) && !hyperLinkException);
+                removeAll(type, [nViewId, hyperLinkException] (const CallbackData& elemData) {
+                        return (nViewId == lcl_getViewId(elemData) && !hyperLinkException);
                     }
                 );
             }
@@ -1629,9 +1632,8 @@ void CallbackFlushHandler::queue(const int type, const char* data)
                     // a save occurs while a cell is still edited in Calc.
                     if (name != ".uno:ModifiedStatus=")
                     {
-                        removeAll(
-                            [type, &name] (int elemType, const CallbackData& elemData) {
-                                return (elemType == type) && (elemData.PayloadString.compare(0, name.size(), name) == 0);
+                        removeAll(type, [&name] (const CallbackData& elemData) {
+                                return (elemData.PayloadString.compare(0, name.size(), name) == 0);
                             }
                         );
                     }
@@ -1648,8 +1650,8 @@ void CallbackFlushHandler::queue(const int type, const char* data)
             {
                 // remove only selection ranges and 'EMPTY' messages
                 // always send 'INPLACE' and 'INPLACE EXIT' messages
-                removeAll([type, payload] (int elemType, const CallbackData& elemData)
-                    { return (elemType == type && elemData.PayloadString[0] != 'I'); });
+                removeAll(type, [payload] (const CallbackData& elemData)
+                    { return (elemData.PayloadString[0] != 'I'); });
             }
             break;
         }
@@ -1734,16 +1736,9 @@ bool CallbackFlushHandler::processInvalidateTilesEvent(int type, CallbackData& a
     {
         SAL_INFO("lok", "Have Empty [" << type << "]: [" << payload
                                        << "] so removing all with part " << rcNew.m_nPart << ".");
-        removeAll([&rcNew](int elemType, const CallbackData& elemData) {
-            if (elemType == LOK_CALLBACK_INVALIDATE_TILES)
-            {
-                // Remove exiting if new is all-encompassing, or if of the same part.
-                const RectangleAndPart rcOld = RectangleAndPart::Create(elemData.PayloadString);
-                return (rcNew.m_nPart == -1 || rcOld.m_nPart == rcNew.m_nPart);
-            }
-
-            // Keep others.
-            return false;
+        removeAll(LOK_CALLBACK_INVALIDATE_TILES, [&rcNew](const CallbackData& elemData) {
+            // Remove exiting if new is all-encompassing, or if of the same part.
+            return (rcNew.m_nPart == -1 || rcNew.m_nPart == elemData.getRectangleAndPart().m_nPart);
         });
     }
     else
@@ -1751,55 +1746,52 @@ bool CallbackFlushHandler::processInvalidateTilesEvent(int type, CallbackData& a
         const auto rcOrig = rcNew;
 
         SAL_INFO("lok", "Have [" << type << "]: [" << payload << "] so merging overlapping.");
-        removeAll([&rcNew](int elemType, const CallbackData& elemData) {
-            if (elemType == LOK_CALLBACK_INVALIDATE_TILES)
+        removeAll(LOK_CALLBACK_INVALIDATE_TILES,[&rcNew](const CallbackData& elemData) {
+            const RectangleAndPart& rcOld = elemData.getRectangleAndPart();
+            if (rcNew.m_nPart != -1 && rcOld.m_nPart != -1 && rcOld.m_nPart != rcNew.m_nPart)
             {
-                const RectangleAndPart& rcOld = elemData.getRectangleAndPart();
-                if (rcNew.m_nPart != -1 && rcOld.m_nPart != -1 && rcOld.m_nPart != rcNew.m_nPart)
-                {
-                    SAL_INFO("lok", "Nothing to merge between new: "
-                                        << rcNew.toString() << ", and old: " << rcOld.toString());
-                    return false;
-                }
+                SAL_INFO("lok", "Nothing to merge between new: "
+                                    << rcNew.toString() << ", and old: " << rcOld.toString());
+                return false;
+            }
 
-                if (rcNew.m_nPart == -1)
+            if (rcNew.m_nPart == -1)
+            {
+                // Don't merge unless fully overlapped.
+                SAL_INFO("lok", "New " << rcNew.toString() << " has " << rcOld.toString()
+                                       << "?");
+                if (rcNew.m_aRectangle.Contains(rcOld.m_aRectangle))
                 {
-                    // Don't merge unless fully overlapped.
-                    SAL_INFO("lok", "New " << rcNew.toString() << " has " << rcOld.toString()
-                                           << "?");
-                    if (rcNew.m_aRectangle.Contains(rcOld.m_aRectangle))
-                    {
-                        SAL_INFO("lok", "New " << rcNew.toString() << " engulfs old "
-                                               << rcOld.toString() << ".");
-                        return true;
-                    }
+                    SAL_INFO("lok", "New " << rcNew.toString() << " engulfs old "
+                                           << rcOld.toString() << ".");
+                    return true;
                 }
-                else if (rcOld.m_nPart == -1)
+            }
+            else if (rcOld.m_nPart == -1)
+            {
+                // Don't merge unless fully overlapped.
+                SAL_INFO("lok", "Old " << rcOld.toString() << " has " << rcNew.toString()
+                                       << "?");
+                if (rcOld.m_aRectangle.Contains(rcNew.m_aRectangle))
                 {
-                    // Don't merge unless fully overlapped.
-                    SAL_INFO("lok", "Old " << rcOld.toString() << " has " << rcNew.toString()
-                                           << "?");
-                    if (rcOld.m_aRectangle.Contains(rcNew.m_aRectangle))
-                    {
-                        SAL_INFO("lok", "New " << rcNew.toString() << " engulfs old "
-                                               << rcOld.toString() << ".");
-                        return true;
-                    }
+                    SAL_INFO("lok", "New " << rcNew.toString() << " engulfs old "
+                                           << rcOld.toString() << ".");
+                    return true;
                 }
-                else
+            }
+            else
+            {
+                const tools::Rectangle rcOverlap
+                    = rcNew.m_aRectangle.GetIntersection(rcOld.m_aRectangle);
+                const bool bOverlap = !rcOverlap.IsEmpty();
+                SAL_INFO("lok", "Merging " << rcNew.toString() << " & " << rcOld.toString()
+                                           << " => " << rcOverlap.toString()
+                                           << " Overlap: " << bOverlap);
+                if (bOverlap)
                 {
-                    const tools::Rectangle rcOverlap
-                        = rcNew.m_aRectangle.GetIntersection(rcOld.m_aRectangle);
-                    const bool bOverlap = !rcOverlap.IsEmpty();
-                    SAL_INFO("lok", "Merging " << rcNew.toString() << " & " << rcOld.toString()
-                                               << " => " << rcOverlap.toString()
-                                               << " Overlap: " << bOverlap);
-                    if (bOverlap)
-                    {
-                        rcNew.m_aRectangle.Union(rcOld.m_aRectangle);
-                        SAL_INFO("lok", "Merged: " << rcNew.toString());
-                        return true;
-                    }
+                    rcNew.m_aRectangle.Union(rcOld.m_aRectangle);
+                    SAL_INFO("lok", "Merged: " << rcNew.toString());
+                    return true;
                 }
             }
 
@@ -1837,15 +1829,12 @@ bool CallbackFlushHandler::processWindowEvent(int type, CallbackData& aCallbackD
         // remove all previous window part invalidations
         if (aRectStr.empty())
         {
-            removeAll([&nLOKWindowId](int elemType, const CallbackData& elemData) {
-                if (elemType == LOK_CALLBACK_WINDOW)
+            removeAll(LOK_CALLBACK_WINDOW,[&nLOKWindowId](const CallbackData& elemData) {
+                const boost::property_tree::ptree& aOldTree = elemData.getJson();
+                if (nLOKWindowId == aOldTree.get<unsigned>("id", 0)
+                    && aOldTree.get<std::string>("action", "") == "invalidate")
                 {
-                    const boost::property_tree::ptree& aOldTree = elemData.getJson();
-                    if (nLOKWindowId == aOldTree.get<unsigned>("id", 0)
-                        && aOldTree.get<std::string>("action", "") == "invalidate")
-                    {
-                        return true;
-                    }
+                    return true;
                 }
                 return false;
             });
@@ -1886,11 +1875,8 @@ bool CallbackFlushHandler::processWindowEvent(int type, CallbackData& aCallbackD
             aRectStream >> nLeft >> nComma >> nTop >> nComma >> nWidth >> nComma >> nHeight;
             tools::Rectangle aNewRect(nLeft, nTop, nLeft + nWidth, nTop + nHeight);
             bool currentIsRedundant = false;
-            removeAll([&aNewRect, &nLOKWindowId,
-                       &currentIsRedundant](int elemType, const CallbackData& elemData) {
-                if (elemType != LOK_CALLBACK_WINDOW)
-                    return false;
-
+            removeAll(LOK_CALLBACK_WINDOW, [&aNewRect, &nLOKWindowId,
+                       &currentIsRedundant](const CallbackData& elemData) {
                 const boost::property_tree::ptree& aOldTree = elemData.getJson();
                 if (aOldTree.get<std::string>("action", "") == "invalidate")
                 {
@@ -1962,13 +1948,10 @@ bool CallbackFlushHandler::processWindowEvent(int type, CallbackData& aCallbackD
     else if (aAction == "created")
     {
         // Remove all previous actions on same dialog, if we are creating it anew.
-        removeAll([&nLOKWindowId](int elemType, const CallbackData& elemData) {
-            if (elemType == LOK_CALLBACK_WINDOW)
-            {
-                const boost::property_tree::ptree& aOldTree = elemData.getJson();
-                if (nLOKWindowId == aOldTree.get<unsigned>("id", 0))
-                    return true;
-            }
+        removeAll(LOK_CALLBACK_WINDOW,[&nLOKWindowId](const CallbackData& elemData) {
+            const boost::property_tree::ptree& aOldTree = elemData.getJson();
+            if (nLOKWindowId == aOldTree.get<unsigned>("id", 0))
+                return true;
             return false;
         });
 
@@ -1990,16 +1973,13 @@ bool CallbackFlushHandler::processWindowEvent(int type, CallbackData& aCallbackD
     {
         // A size change is practically re-creation of the window.
         // But at a minimum it's a full invalidation.
-        removeAll([&nLOKWindowId](int elemType, const CallbackData& elemData) {
-            if (elemType == LOK_CALLBACK_WINDOW)
+        removeAll(LOK_CALLBACK_WINDOW, [&nLOKWindowId](const CallbackData& elemData) {
+            const boost::property_tree::ptree& aOldTree = elemData.getJson();
+            if (nLOKWindowId == aOldTree.get<unsigned>("id", 0))
             {
-                const boost::property_tree::ptree& aOldTree = elemData.getJson();
-                if (nLOKWindowId == aOldTree.get<unsigned>("id", 0))
-                {
-                    const std::string aOldAction = aOldTree.get<std::string>("action", "");
-                    if (aOldAction == "invalidate")
-                        return true;
-                }
+                const std::string aOldAction = aOldTree.get<std::string>("action", "");
+                if (aOldAction == "invalidate")
+                    return true;
             }
             return false;
         });
@@ -2077,6 +2057,44 @@ void CallbackFlushHandler::Invoke()
     m_queue2.clear();
 }
 
+bool CallbackFlushHandler::removeAll(int type)
+{
+    bool bErased = false;
+    auto it1 = m_queue1.begin();
+    for(;;)
+    {
+        it1 = std::find(it1, m_queue1.end(), type);
+        if(it1 == m_queue1.end())
+            break;
+        m_queue2.erase(toQueue2(it1));
+        it1 = m_queue1.erase(it1);
+        bErased = true;
+    }
+    return bErased;
+}
+
+bool CallbackFlushHandler::removeAll(int type, const std::function<bool (const CallbackData&)>& rTestFunc)
+{
+    bool bErased = false;
+    auto it1 = m_queue1.begin();
+    for(;;)
+    {
+        it1 = std::find(it1, m_queue1.end(), type);
+        if(it1 == m_queue1.end())
+            break;
+        auto it2 = toQueue2(it1);
+        if (rTestFunc(*it2))
+        {
+            m_queue2.erase(it2);
+            it1 = m_queue1.erase(it1);
+            bErased = true;
+        }
+        else
+            ++it1;
+    }
+    return bErased;
+}
+
 bool CallbackFlushHandler::removeAll(const std::function<bool (int, const CallbackData&)>& rTestFunc)
 {
     bool bErased = false;