desktop/inc/lib/init.hxx    |   23 ++---
 desktop/source/lib/init.cxx |  192 +++++++++++++++++++++++++-------------------
 2 files changed, 123 insertions(+), 92 deletions(-)

New commits:
commit 475d6528a5ae1781c5b09cbcd3fd305c4180c168
Author:     Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Wed Aug 4 13:01:22 2021 +0200
Commit:     Luboš Luňák <l.lu...@collabora.com>
CommitDate: Thu Sep 23 00:09:00 2021 +0200

    speed up scanning the LOK queue
    
    we frequently scan the queue to caolesce events.
    Most of the time we are scanning based on the event type.
    So split the queue data into a compact queue that only contains the
    type, and another queue for the rest of the data.
    That makes the scanning __much__ more cache-friendly.
    
    Change-Id: I92d0b95611cd139cac8532f9297eaabda71d5fe9
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119996
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
    (cherry picked from commit acf9cf33d53e4bf598ddbdab102bfbd6bb14f8a3)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121558
    Tested-by: Jenkins
    (cherry picked from commit 3b3e4ee97af23f210fa39f1af3ddf1de63291371)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122434
    Reviewed-by: Luboš Luňák <l.lu...@collabora.com>

diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx
index 3d95d4f602b4..ed35ebc2fa13 100644
--- a/desktop/inc/lib/init.hxx
+++ b/desktop/inc/lib/init.hxx
@@ -92,9 +92,8 @@ namespace desktop {
 
         struct CallbackData
         {
-            CallbackData(int type, const std::string& payload)
-                : Type(type)
-                , PayloadString(payload)
+            CallbackData(const std::string& payload)
+                : PayloadString(payload)
             {
             }
 
@@ -117,7 +116,6 @@ namespace desktop {
             /// Returns true iff there is cached data.
             bool isCached() const { return PayloadObject.which() != 0; }
 
-            int Type;
             std::string PayloadString;
 
         private:
@@ -125,14 +123,19 @@ namespace desktop {
             boost::variant<boost::blank, RectangleAndPart, 
boost::property_tree::ptree> PayloadObject;
         };
 
-        typedef std::vector<CallbackData> queue_type;
+        typedef std::vector<int> queue_type1;
+        typedef std::vector<CallbackData> queue_type2;
 
     private:
-        bool removeAll(const std::function<bool (const 
queue_type::value_type&)>& rTestFunc);
-        bool processInvalidateTilesEvent(CallbackData& aCallbackData);
-        bool processWindowEvent(CallbackData& aCallbackData);
-
-        queue_type m_queue;
+        bool removeAll(const std::function<bool (int, const CallbackData&)>& 
rTestFunc);
+        bool processInvalidateTilesEvent(int type, CallbackData& 
aCallbackData);
+        bool processWindowEvent(int type, CallbackData& aCallbackData);
+        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
+            so we split the queue in 2 to make the scanning cache friendly. */
+        queue_type1 m_queue1;
+        queue_type2 m_queue2;
         std::map<int, std::string> m_states;
         std::unordered_map<int, std::unordered_map<int, std::string>> 
m_viewStates;
         LibreOfficeKitDocument* m_pDocument;
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index c95de6ed4082..86c468df402f 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1438,13 +1438,19 @@ void CallbackFlushHandler::callback(const int type, 
const char* payload, void* d
     }
 }
 
+CallbackFlushHandler::queue_type2::reverse_iterator 
CallbackFlushHandler::toQueue2(CallbackFlushHandler::queue_type1::reverse_iterator
 pos)
+{
+    int delta = std::distance(m_queue1.rbegin(), pos);
+    return m_queue2.rbegin() + delta;
+}
+
 void CallbackFlushHandler::queue(const int type, const char* data)
 {
     comphelper::ProfileZone aZone("CallbackFlushHandler::queue");
 
-    CallbackData aCallbackData(type, (data ? data : "(nil)"));
+    CallbackData aCallbackData(data ? data : "(nil)");
     const std::string& payload = aCallbackData.PayloadString;
-    SAL_INFO("lok", "Queue: [" << type << "]: [" << payload << "] on " << 
m_queue.size() << " entries.");
+    SAL_INFO("lok", "Queue: [" << type << "]: [" << payload << "] on " << 
m_queue1.size() << " entries.");
 
     bool bIsChartActive = false;
     if (type == LOK_CALLBACK_GRAPHIC_SELECTION)
@@ -1523,10 +1529,10 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
         case LOK_CALLBACK_CALC_FUNCTION_LIST:
         case LOK_CALLBACK_INVALIDATE_SHEET_GEOMETRY:
         {
-            const auto& pos = std::find_if(m_queue.rbegin(), m_queue.rend(),
-                    [type] (const queue_type::value_type& elem) { return 
(elem.Type == type); });
-
-            if (pos != m_queue.rend() && pos->PayloadString == payload)
+            const auto& pos = std::find_if(m_queue1.rbegin(), m_queue1.rend(),
+                    [type] (int elemType) { return (elemType == type); });
+            auto pos2 = toQueue2(pos);
+            if (pos != m_queue1.rend() && pos2->PayloadString == payload)
             {
                 SAL_INFO("lok", "Skipping queue duplicate [" << type << + "]: 
[" << payload << "].");
                 return;
@@ -1537,15 +1543,17 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
 
     if (type == LOK_CALLBACK_TEXT_SELECTION && payload.empty())
     {
-        const auto& posStart = std::find_if(m_queue.rbegin(), m_queue.rend(),
-                [] (const queue_type::value_type& elem) { return (elem.Type == 
LOK_CALLBACK_TEXT_SELECTION_START); });
-        if (posStart != m_queue.rend())
-            posStart->PayloadString.clear();
+        const auto& posStart = std::find_if(m_queue1.rbegin(), m_queue1.rend(),
+                [] (int elemType) { return (elemType == 
LOK_CALLBACK_TEXT_SELECTION_START); });
+        auto posStart2 = toQueue2(posStart);
+        if (posStart != m_queue1.rend())
+            posStart2->PayloadString.clear();
 
-        const auto& posEnd = std::find_if(m_queue.rbegin(), m_queue.rend(),
-                [] (const queue_type::value_type& elem) { return (elem.Type == 
LOK_CALLBACK_TEXT_SELECTION_END); });
-        if (posEnd != m_queue.rend())
-            posEnd->PayloadString.clear();
+        const auto& posEnd = std::find_if(m_queue1.rbegin(), m_queue1.rend(),
+                [] (int elemType) { return (elemType == 
LOK_CALLBACK_TEXT_SELECTION_END); });
+        auto posEnd2 = toQueue2(posEnd);
+        if (posEnd != m_queue1.rend())
+            posEnd2->PayloadString.clear();
     }
 
     // When payload is empty discards any previous state.
@@ -1560,7 +1568,7 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
             case LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR:
             case LOK_CALLBACK_INVALIDATE_TILES:
                 if (removeAll(
-                        [type](const queue_type::value_type& elem) { return 
(elem.Type == type); }))
+                        [type](int elemType, const CallbackData&) { return 
(elemType == type); }))
                     SAL_INFO("lok", "Removed dups of [" << type << "]: [" << 
payload << "].");
                 break;
         }
@@ -1584,7 +1592,7 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
             case LOK_CALLBACK_RULER_UPDATE:
             {
                 if (removeAll(
-                        [type](const queue_type::value_type& elem) { return 
(elem.Type == type); }))
+                        [type](int elemType, const CallbackData&) { return 
(elemType == type); }))
                     SAL_INFO("lok", "Removed dups of [" << type << "]: [" << 
payload << "].");
             }
             break;
@@ -1608,15 +1616,15 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
                     payload.find("\"hyperlink\": {}") == std::string::npos;
                 const int nViewId = lcl_getViewId(payload);
                 removeAll(
-                    [type, nViewId, hyperLinkException] (const 
queue_type::value_type& elem) {
-                        return (elem.Type == type && nViewId == 
lcl_getViewId(elem) && !hyperLinkException);
+                    [type, nViewId, hyperLinkException] (int elemType, const 
CallbackData& elemData) {
+                        return (elemType == type && nViewId == 
lcl_getViewId(elemData) && !hyperLinkException);
                     }
                 );
             }
             break;
 
             case LOK_CALLBACK_INVALIDATE_TILES:
-                if (processInvalidateTilesEvent(aCallbackData))
+                if (processInvalidateTilesEvent(type, aCallbackData))
                     return;
             break;
 
@@ -1634,8 +1642,8 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
                     if (name != ".uno:ModifiedStatus=")
                     {
                         removeAll(
-                            [type, &name] (const queue_type::value_type& elem) 
{
-                                return (elem.Type == type) && 
(elem.PayloadString.compare(0, name.size(), name) == 0);
+                            [type, &name] (int elemType, const CallbackData& 
elemData) {
+                                return (elemType == type) && 
(elemData.PayloadString.compare(0, name.size(), name) == 0);
                             }
                         );
                     }
@@ -1644,7 +1652,7 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
             break;
 
             case LOK_CALLBACK_WINDOW:
-                if (processWindowEvent(aCallbackData))
+                if (processWindowEvent(type, aCallbackData))
                     return;
             break;
 
@@ -1652,8 +1660,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] (const queue_type::value_type& elem)
-                    { return (elem.Type == type && elem.PayloadString[0] != 
'I'); });
+                removeAll([type, payload] (int elemType, const CallbackData& 
elemData)
+                    { return (elemType == type && elemData.PayloadString[0] != 
'I'); });
             }
             break;
         }
@@ -1661,25 +1669,28 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
 
     // Validate that the cached data and the payload string are identical.
     assert(aCallbackData.validate() && "Cached callback payload object and 
string mismatch!");
-    m_queue.emplace_back(aCallbackData);
-    SAL_INFO("lok", "Queued #" << (m_queue.size() - 1) <<
-             " [" << type << "]: [" << payload << "] to have " << 
m_queue.size() << " entries.");
+    m_queue1.emplace_back(type);
+    m_queue2.emplace_back(aCallbackData);
+    SAL_INFO("lok", "Queued #" << (m_queue1.size() - 1) <<
+             " [" << type << "]: [" << payload << "] to have " << 
m_queue1.size() << " entries.");
 
 #ifdef DBG_UTIL
     {
         // Dump the queue state and validate cached data.
         int i = 1;
         std::ostringstream oss;
-        if (m_queue.empty())
+        if (m_queue1.empty())
             oss << "Empty";
         else
-            oss << m_queue.size() << " items\n";
-        for (const CallbackData& c : m_queue)
-            oss << i++ << ": [" << c.Type << "] [" << c.PayloadString << 
"].\n";
+            oss << m_queue1.size() << " items\n";
+        auto it1 = m_queue1.begin();
+        auto it2 = m_queue2.begin();
+        for (; it1 != m_queue1.end(); ++it1, ++it2)
+            oss << i++ << ": [" << *it1 << "] [" << it2->PayloadString << 
"].\n";
         SAL_INFO("lok", "Current Queue: " << oss.str());
         assert(
             std::all_of(
-                m_queue.begin(), m_queue.end(),
+                m_queue2.begin(), m_queue2.end(),
                 [](const CallbackData& c) { return c.validate(); }));
     }
 #endif
@@ -1691,10 +1702,9 @@ void CallbackFlushHandler::queue(const int type, const 
char* data)
     }
 }
 
-bool CallbackFlushHandler::processInvalidateTilesEvent(CallbackData& 
aCallbackData)
+bool CallbackFlushHandler::processInvalidateTilesEvent(int type, CallbackData& 
aCallbackData)
 {
     const std::string& payload = aCallbackData.PayloadString;
-    const int type = aCallbackData.Type;
 
     RectangleAndPart& rcNew = aCallbackData.setRectangleAndPart(payload);
     if (rcNew.isEmpty())
@@ -1706,12 +1716,13 @@ bool 
CallbackFlushHandler::processInvalidateTilesEvent(CallbackData& aCallbackDa
     // If we have to invalidate all tiles, we can skip any new tile 
invalidation.
     // Find the last INVALIDATE_TILES entry, if any to see if it's 
invalidate-all.
     const auto& pos
-        = std::find_if(m_queue.rbegin(), m_queue.rend(), [](const 
queue_type::value_type& elem) {
-              return (elem.Type == LOK_CALLBACK_INVALIDATE_TILES);
+        = std::find_if(m_queue1.rbegin(), m_queue1.rend(), [](int elemType) {
+              return (elemType == LOK_CALLBACK_INVALIDATE_TILES);
           });
-    if (pos != m_queue.rend())
+    if (pos != m_queue1.rend())
     {
-        const RectangleAndPart& rcOld = pos->getRectangleAndPart();
+        auto pos2 = toQueue2(pos);
+        const RectangleAndPart& rcOld = pos2->getRectangleAndPart();
         if (rcOld.isInfinite() && (rcOld.m_nPart == -1 || rcOld.m_nPart == 
rcNew.m_nPart))
         {
             SAL_INFO("lok", "Skipping queue [" << type << "]: [" << payload
@@ -1735,11 +1746,11 @@ bool 
CallbackFlushHandler::processInvalidateTilesEvent(CallbackData& aCallbackDa
     {
         SAL_INFO("lok", "Have Empty [" << type << "]: [" << payload
                                        << "] so removing all with part " << 
rcNew.m_nPart << ".");
-        removeAll([&rcNew](const queue_type::value_type& elem) {
-            if (elem.Type == LOK_CALLBACK_INVALIDATE_TILES)
+        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(elem.PayloadString);
+                const RectangleAndPart rcOld = 
RectangleAndPart::Create(elemData.PayloadString);
                 return (rcNew.m_nPart == -1 || rcOld.m_nPart == rcNew.m_nPart);
             }
 
@@ -1752,10 +1763,10 @@ bool 
CallbackFlushHandler::processInvalidateTilesEvent(CallbackData& aCallbackDa
         const auto rcOrig = rcNew;
 
         SAL_INFO("lok", "Have [" << type << "]: [" << payload << "] so merging 
overlapping.");
-        removeAll([&rcNew](const queue_type::value_type& elem) {
-            if (elem.Type == LOK_CALLBACK_INVALIDATE_TILES)
+        removeAll([&rcNew](int elemType, const CallbackData& elemData) {
+            if (elemType == LOK_CALLBACK_INVALIDATE_TILES)
             {
-                const RectangleAndPart& rcOld = elem.getRectangleAndPart();
+                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: "
@@ -1824,10 +1835,9 @@ bool 
CallbackFlushHandler::processInvalidateTilesEvent(CallbackData& aCallbackDa
     return false;
 }
 
-bool CallbackFlushHandler::processWindowEvent(CallbackData& aCallbackData)
+bool CallbackFlushHandler::processWindowEvent(int type, CallbackData& 
aCallbackData)
 {
     const std::string& payload = aCallbackData.PayloadString;
-    const int type = aCallbackData.Type;
 
     boost::property_tree::ptree& aTree = aCallbackData.setJson(payload);
     const unsigned nLOKWindowId = aTree.get<unsigned>("id", 0);
@@ -1839,10 +1849,10 @@ bool 
CallbackFlushHandler::processWindowEvent(CallbackData& aCallbackData)
         // remove all previous window part invalidations
         if (aRectStr.empty())
         {
-            removeAll([&nLOKWindowId](const queue_type::value_type& elem) {
-                if (elem.Type == LOK_CALLBACK_WINDOW)
+            removeAll([&nLOKWindowId](int elemType, const CallbackData& 
elemData) {
+                if (elemType == LOK_CALLBACK_WINDOW)
                 {
-                    const boost::property_tree::ptree& aOldTree = 
elem.getJson();
+                    const boost::property_tree::ptree& aOldTree = 
elemData.getJson();
                     if (nLOKWindowId == aOldTree.get<unsigned>("id", 0)
                         && aOldTree.get<std::string>("action", "") == 
"invalidate")
                     {
@@ -1856,17 +1866,22 @@ bool 
CallbackFlushHandler::processWindowEvent(CallbackData& aCallbackData)
         {
             // if we have to invalidate all of the window, ignore
             // any part invalidation message
-            const auto invAllExist = std::any_of(m_queue.rbegin(), 
m_queue.rend(),
-                                        [&nLOKWindowId] (const 
queue_type::value_type& elem)
-                                        {
-                                            if (elem.Type != 
LOK_CALLBACK_WINDOW)
-                                                return false;
-
-                                            const boost::property_tree::ptree& 
aOldTree = elem.getJson();
-                                            return nLOKWindowId == 
aOldTree.get<unsigned>("id", 0)
-                                                && 
aOldTree.get<std::string>("action", "") == "invalidate"
-                                                && 
aOldTree.get<std::string>("rectangle", "").empty();
-                                        });
+            bool invAllExist = false;
+            auto it1 = m_queue1.rbegin();
+            auto it2 = m_queue2.rbegin();
+            for (;it1 != m_queue1.rend(); ++it1, ++it2)
+            {
+                if (*it1 != LOK_CALLBACK_WINDOW)
+                    continue;
+                const boost::property_tree::ptree& aOldTree = it2->getJson();
+                if (nLOKWindowId == aOldTree.get<unsigned>("id", 0)
+                     && aOldTree.get<std::string>("action", "") == "invalidate"
+                     && aOldTree.get<std::string>("rectangle", "").empty())
+                {
+                    invAllExist = true;
+                    break;
+                }
+            }
 
             // we found a invalidate-all window callback
             if (invAllExist)
@@ -1884,11 +1899,11 @@ bool 
CallbackFlushHandler::processWindowEvent(CallbackData& aCallbackData)
             tools::Rectangle aNewRect(nLeft, nTop, nLeft + nWidth, nTop + 
nHeight);
             bool currentIsRedundant = false;
             removeAll([&aNewRect, &nLOKWindowId,
-                       &currentIsRedundant](const queue_type::value_type& 
elem) {
-                if (elem.Type != LOK_CALLBACK_WINDOW)
+                       &currentIsRedundant](int elemType, const CallbackData& 
elemData) {
+                if (elemType != LOK_CALLBACK_WINDOW)
                     return false;
 
-                const boost::property_tree::ptree& aOldTree = elem.getJson();
+                const boost::property_tree::ptree& aOldTree = 
elemData.getJson();
                 if (aOldTree.get<std::string>("action", "") == "invalidate")
                 {
                     // Not possible that we encounter an empty rectangle here; 
we already handled this case above.
@@ -1959,10 +1974,10 @@ bool 
CallbackFlushHandler::processWindowEvent(CallbackData& aCallbackData)
     else if (aAction == "created")
     {
         // Remove all previous actions on same dialog, if we are creating it 
anew.
-        removeAll([&nLOKWindowId](const queue_type::value_type& elem) {
-            if (elem.Type == LOK_CALLBACK_WINDOW)
+        removeAll([&nLOKWindowId](int elemType, const CallbackData& elemData) {
+            if (elemType == LOK_CALLBACK_WINDOW)
             {
-                const boost::property_tree::ptree& aOldTree = elem.getJson();
+                const boost::property_tree::ptree& aOldTree = 
elemData.getJson();
                 if (nLOKWindowId == aOldTree.get<unsigned>("id", 0))
                     return true;
             }
@@ -1987,10 +2002,10 @@ bool 
CallbackFlushHandler::processWindowEvent(CallbackData& aCallbackData)
     {
         // A size change is practically re-creation of the window.
         // But at a minimum it's a full invalidation.
-        removeAll([&nLOKWindowId](const queue_type::value_type& elem) {
-            if (elem.Type == LOK_CALLBACK_WINDOW)
+        removeAll([&nLOKWindowId](int elemType, const CallbackData& elemData) {
+            if (elemType == LOK_CALLBACK_WINDOW)
             {
-                const boost::property_tree::ptree& aOldTree = elem.getJson();
+                const boost::property_tree::ptree& aOldTree = 
elemData.getJson();
                 if (nLOKWindowId == aOldTree.get<unsigned>("id", 0))
                 {
                     const std::string aOldAction = 
aOldTree.get<std::string>("action", "");
@@ -2015,12 +2030,14 @@ void CallbackFlushHandler::Invoke()
 
     std::scoped_lock<std::mutex> lock(m_mutex);
 
-    SAL_INFO("lok", "Flushing " << m_queue.size() << " elements.");
-    for (const auto& rCallbackData : m_queue)
+    SAL_INFO("lok", "Flushing " << m_queue1.size() << " elements.");
+    auto it1 = m_queue1.begin();
+    auto it2 = m_queue2.begin();
+    for (; it1 != m_queue1.end(); ++it1, ++it2)
     {
-        const int type = rCallbackData.Type;
-        const auto& payload = rCallbackData.PayloadString;
-        const int viewId = lcl_isViewCallbackType(type) ? 
lcl_getViewId(rCallbackData) : -1;
+        const int type = *it1;
+        const auto& payload = it2->PayloadString;
+        const int viewId = lcl_isViewCallbackType(type) ? lcl_getViewId(*it2) 
: -1;
 
         if (viewId == -1)
         {
@@ -2068,19 +2085,30 @@ void CallbackFlushHandler::Invoke()
         m_pCallback(type, payload.c_str(), m_pData);
     }
 
-    m_queue.clear();
+    m_queue1.clear();
+    m_queue2.clear();
 }
 
-bool CallbackFlushHandler::removeAll(const std::function<bool (const 
CallbackFlushHandler::queue_type::value_type&)>& rTestFunc)
+bool CallbackFlushHandler::removeAll(const std::function<bool (int, const 
CallbackData&)>& rTestFunc)
 {
-    auto newEnd = std::remove_if(m_queue.begin(), m_queue.end(), rTestFunc);
-    if (newEnd != m_queue.end())
+    bool bErased = false;
+    auto it1 = m_queue1.begin();
+    auto it2 = m_queue2.begin();
+    while (it1 != m_queue1.end())
     {
-        m_queue.erase(newEnd, m_queue.end());
-        return true;
+        if (rTestFunc(*it1, *it2))
+        {
+            it1 = m_queue1.erase(it1);
+            it2 = m_queue2.erase(it2);
+            bErased = true;
+        }
+        else
+        {
+            ++it1;
+            ++it2;
+        }
     }
-
-    return false;
+    return bErased;
 }
 
 void CallbackFlushHandler::addViewStates(int viewId)

Reply via email to