[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-1-9' - 4 commits - loleaflet/src loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLKit.cpp loolwsd/MessageQueue.cpp loolwsd/PrisonerSession.cpp loolwsd/protocol.txt loolwsd/TileCache.cpp loolwsd/TileDesc.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Thu Sep 1 21:44:02 UTC 2016


 loleaflet/src/layer/tile/TileLayer.js |    2 -
 loolwsd/DocumentBroker.cpp            |   17 +++++----
 loolwsd/DocumentBroker.hpp            |    6 ++-
 loolwsd/LOOLKit.cpp                   |   59 +++++++++++++++++++++++++++++++---
 loolwsd/MessageQueue.cpp              |   43 ++++++++++++++++--------
 loolwsd/PrisonerSession.cpp           |   10 ++++-
 loolwsd/TileCache.cpp                 |    4 +-
 loolwsd/TileDesc.hpp                  |    7 ++++
 loolwsd/protocol.txt                  |    4 +-
 9 files changed, 118 insertions(+), 34 deletions(-)

New commits:
commit 1b61e1d52fd950e9969ee85fe98af50426a8efdf
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Thu Sep 1 16:45:04 2016 -0400

    loolwsd: replace old tile requests with more recent ones
    
    A new queue with own thread now processes tile requests
    so we have a chance to cull backlogged tile requests
    that have since been invalidated.
    
    This reduces load on the server significantly, especially
    with multiple view at different zoom levels, and very fast
    typing.
    
    Change-Id: I6599939cd363dc71c30187f40d542aa37260dc56
    Reviewed-on: https://gerrit.libreoffice.org/28607
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 0613aa7..b62754d 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -27,6 +27,7 @@
 #include <iostream>
 #include <memory>
 #include <sstream>
+#include <thread>
 
 #define LOK_USE_UNSTABLE_API
 #include <LibreOfficeKit/LibreOfficeKitInit.h>
@@ -396,18 +397,23 @@ public:
     Document(const std::shared_ptr<lok::Office>& loKit,
              const std::string& jailId,
              const std::string& docKey,
-             const std::string& url)
+             const std::string& url,
+             std::shared_ptr<TileQueue> queue,
+             const std::shared_ptr<WebSocket>& ws)
       : _multiView(std::getenv("LOK_VIEW_CALLBACK")),
         _loKit(loKit),
         _jailId(jailId),
         _docKey(docKey),
         _url(url),
+        _queue(queue),
+        _ws(ws),
         _docPassword(""),
         _haveDocPassword(false),
         _isDocPasswordProtected(false),
         _docPasswordType(PasswordType::ToView),
         _stop(false),
         _isLoading(0),
+        _tilesThread(tilesThread, this),
         _clientViews(0)
     {
         Log::info("Document ctor for url [" + _url + "] on child [" + _jailId +
@@ -1188,6 +1194,44 @@ private:
         Log::debug("Thread finished.");
     }
 
+    static void tilesThread(Document* pThis)
+    {
+        Util::setThreadName("tile_renderer");
+
+        Log::debug("Thread started.");
+
+        try
+        {
+            while (true)
+            {
+                const auto input = pThis->_queue->get();
+                const std::string message(input.data(), input.size());
+                StringTokenizer tokens(message, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
+
+                if (tokens[0] == "eof")
+                {
+                    Log::info("Received EOF. Finishing.");
+                    break;
+                }
+
+                if (tokens[0] == "tile")
+                {
+                    pThis->renderTile(tokens, pThis->_ws);
+                }
+                else if (tokens[0] == "tilecombine")
+                {
+                    pThis->renderCombinedTiles(tokens, pThis->_ws);
+                }
+            }
+        }
+        catch (const std::exception& exc)
+        {
+            Log::error(std::string("QueueHandler::run: Exception: ") + exc.what());
+        }
+
+        Log::debug("Thread finished.");
+    }
+
 private:
 
     const bool _multiView;
@@ -1199,6 +1243,8 @@ private:
     std::string _renderOpts;
 
     std::shared_ptr<lok::Document> _loKitDocument;
+    std::shared_ptr<TileQueue> _queue;
+    std::shared_ptr<WebSocket> _ws;
 
     // Document password provided
     std::string _docPassword;
@@ -1216,6 +1262,7 @@ private:
     std::map<int, std::unique_ptr<CallbackDescriptor>> _viewIdToCallbackDescr;
     std::map<unsigned, std::shared_ptr<Connection>> _connections;
     Poco::Thread _callbackThread;
+    std::thread _tilesThread;
     Poco::NotificationQueue _callbackQueue;
     std::atomic_size_t _clientViews;
 };
@@ -1420,9 +1467,11 @@ void lokit_main(const std::string& childRoot,
         auto ws = std::make_shared<WebSocket>(cs, request, response);
         ws->setReceiveTimeout(0);
 
+        auto queue = std::make_shared<TileQueue>();
+
         const std::string socketName = "ChildControllerWS";
         IoUtil::SocketProcessor(ws,
-                [&socketName, &ws, &document, &loKit](const std::vector<char>& data)
+                [&socketName, &ws, &document, &loKit, &queue](const std::vector<char>& data)
                 {
                     std::string message(data.data(), data.size());
 
@@ -1449,7 +1498,7 @@ void lokit_main(const std::string& childRoot,
 
                         if (!document)
                         {
-                            document = std::make_shared<Document>(loKit, jailId, docKey, url);
+                            document = std::make_shared<Document>(loKit, jailId, docKey, url, queue, ws);
                         }
 
                         // Validate and create session.
@@ -1463,7 +1512,7 @@ void lokit_main(const std::string& childRoot,
                     {
                         if (document)
                         {
-                            document->renderTile(tokens, ws);
+                            queue->put(message);
                         }
                         else
                         {
@@ -1474,7 +1523,7 @@ void lokit_main(const std::string& childRoot,
                     {
                         if (document)
                         {
-                            document->renderCombinedTiles(tokens, ws);
+                            queue->put(message);
                         }
                         else
                         {
diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp
index 642c7c0..b4a9c20 100644
--- a/loolwsd/MessageQueue.cpp
+++ b/loolwsd/MessageQueue.cpp
@@ -11,6 +11,8 @@
 
 #include <algorithm>
 
+#include <Log.hpp>
+
 MessageQueue::~MessageQueue()
 {
     clear();
@@ -93,24 +95,35 @@ void BasicTileQueue::put_impl(const Payload& value)
 
 void TileQueue::put_impl(const Payload& value)
 {
-    const auto msg = std::string(&value[0], value.size());
-    if (msg.compare(0, 5, "tile ") == 0)
+    if (!_queue.empty())
     {
-        // TODO: implement a real re-ordering here, so that the tiles closest to
-        // the cursor are returned first.
-        // * we will want to put just a general "tile" message to the queue
-        // * add a std::set that handles the tiles
-        // * change the get_impl() to decide which tile is the correct one to
-        //   be returned
-        // * we will also need to be informed about the position of the cursor
-        //   so that get_impl() returns optimal results
-        //
-        // For now: just don't put duplicates into the queue
-        for (const auto& it : _queue)
+        const auto msg = std::string(value.data(), value.size());
+        if (msg.compare(0, 4, "tile") == 0 || msg.compare(0, 10, "tilecombine") == 0)
         {
-            if (value == it)
+            const auto newMsg = msg.substr(0, msg.find(" ver"));
+
+            // TODO: implement a real re-ordering here, so that the tiles closest to
+            // the cursor are returned first.
+            // * we will want to put just a general "tile" message to the queue
+            // * add a std::set that handles the tiles
+            // * change the get_impl() to decide which tile is the correct one to
+            //   be returned
+            // * we will also need to be informed about the position of the cursor
+            //   so that get_impl() returns optimal results
+            //
+            // For now: just don't put duplicates into the queue
+            for (size_t i = 0; i < _queue.size(); ++i)
             {
-                return;
+                auto& it = _queue[i];
+                const std::string old(it.data(), it.size());
+                const auto oldMsg = old.substr(0, old.find(" ver"));
+                Log::error(std::to_string(i) + ": " + oldMsg);
+                if (newMsg == oldMsg)
+                {
+                    Log::trace("Replacing duplicate tile: " + oldMsg + " -> " + newMsg);
+                    _queue[i] = value;
+                    return;
+                }
             }
         }
     }
commit 9d8f100c4b2bf0166f90a9148f1a827b90765797
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Thu Sep 1 21:17:56 2016 +0100

    Send intermediate tiles back even when we they are older.
    
    Should improve interactivity and avoid jerky feedback.

diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index 9b76a15..0f6936a 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -140,17 +140,19 @@ std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile)
     return nullptr;
 }
 
-void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size, const bool priority)
+void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size, const bool /* priority */)
 {
     std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
 
     std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
+#if 0
     if (!priority && tileBeingRendered && tileBeingRendered->getVersion() != tile.getVersion())
     {
         Log::trace() << "Skipping unexpected tile ver: " << tile.getVersion()
                      << ", waiting for ver " << tileBeingRendered->getVersion() << Log::end;
         return;
     }
+#endif
 
     // Save to disk.
     const auto cachedName = (tileBeingRendered ? tileBeingRendered->getCacheName()
commit 407e90f1dc525b9cb39845773a2690ac1f998672
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Thu Sep 1 21:15:13 2016 +0100

    Assign priority based on intersection with the cursor.
    
    Should help to reduce visual tearing when cursor spans a tile join.

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index a89b9d3..043218f 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -111,6 +111,8 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
     _lastEditableSession(false),
     _cursorPosX(0),
     _cursorPosY(0),
+    _cursorWidth(0),
+    _cursorHeight(0),
     _isLoaded(false),
     _isModified(false),
     _isEditLockHeld(false),
@@ -572,8 +574,8 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
                 continue;
             }
             else
-            if (_cursorPosX >= tile.getTilePosX() && _cursorPosX <= tile.getTilePosX() + tile.getTileWidth() &&
-                _cursorPosY >= tile.getTilePosY() && _cursorPosY <= tile.getTilePosY() + tile.getTileHeight())
+            if (tile.intersectsWithRect(_cursorPosX, _cursorPosY,
+                                        _cursorWidth, _cursorHeight))
             {
                 // If this tile is right under the cursor, give it priority.
                 const auto req = tile.serialize("tile");
@@ -628,11 +630,12 @@ void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
         if (firstLine.size() < static_cast<std::string::size_type>(length) - 1)
         {
             // If the tile right under the cursor, give it priority.
-            const auto priority = (_cursorPosX >= tile.getTilePosX() &&
-                                   _cursorPosX <= tile.getTilePosX() + tile.getTileWidth() &&
-                                   _cursorPosY >= tile.getTilePosY() &&
-                                   _cursorPosY <= tile.getTilePosY() + tile.getTileHeight());
-            tileCache().saveTileAndNotify(tile, buffer + firstLine.size() + 1, length - firstLine.size() - 1, priority);
+            const bool priority = tile.intersectsWithRect(
+                _cursorPosX, _cursorPosY,
+                _cursorWidth, _cursorHeight);
+            tileCache().saveTileAndNotify(
+                tile, buffer + firstLine.size() + 1,
+                length - firstLine.size() - 1, priority);
         }
         else
         {
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 54ca138..fd28ed8 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -203,10 +203,12 @@ public:
     size_t removeSession(const std::string& id);
 
     /// Invalidate the cursor position.
-    void invalidateCursor(const int x, const int y)
+    void invalidateCursor(int x, int y, int w, int h)
     {
         _cursorPosX = x;
         _cursorPosY = y;
+        _cursorWidth = w;
+        _cursorHeight = h;
     }
 
     void handleTileRequest(TileDesc& tile,
@@ -252,6 +254,8 @@ private:
     std::atomic<bool> _lastEditableSession;
     int _cursorPosX;
     int _cursorPosY;
+    int _cursorWidth;
+    int _cursorHeight;
     bool _isLoaded;
     bool _isModified;
     mutable std::mutex _mutex;
diff --git a/loolwsd/PrisonerSession.cpp b/loolwsd/PrisonerSession.cpp
index ba3c344..35ce139 100644
--- a/loolwsd/PrisonerSession.cpp
+++ b/loolwsd/PrisonerSession.cpp
@@ -228,13 +228,17 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
         {
             assert(firstLine.size() == static_cast<std::string::size_type>(length));
             StringTokenizer firstLineTokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
-            int x = 0;
-            int y = 0;
+            int x = 0, y = 0, w = 0, h = 0;
             if (firstLineTokens.count() > 2 &&
                 stringToInteger(firstLineTokens[1], x) &&
                 stringToInteger(firstLineTokens[2], y))
             {
-                _docBroker->invalidateCursor(x, y);
+                if (firstLineTokens.count() > 3)
+                {
+                    stringToInteger(firstLineTokens[3], w);
+                    stringToInteger(firstLineTokens[4], h);
+                }
+                _docBroker->invalidateCursor(x, y, w, h);
             }
             else
             {
diff --git a/loolwsd/TileDesc.hpp b/loolwsd/TileDesc.hpp
index 9d5f82c..e4d2c36 100644
--- a/loolwsd/TileDesc.hpp
+++ b/loolwsd/TileDesc.hpp
@@ -60,6 +60,13 @@ public:
     void setVersion(const int ver) { _ver = ver; }
     int getImgSize() const { return _imgSize; }
     void setImgSize(const int imgSize) { _imgSize = imgSize; }
+    bool intersectsWithRect(int x, int y, int w, int h) const
+    {
+        return x + w >= getTilePosX() &&
+               x <= getTilePosX() + getTileWidth() &&
+               y + h >= getTilePosY() &&
+               y <= getTilePosY() + getTileHeight();
+    }
 
     /// Serialize this instance into a string.
     /// Optionally prepend a prefix.
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index 332d088..3639484 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -294,7 +294,9 @@ the client, consisting of the FOO_BAR part in lowercase, without
 underscore, followed by a colon, space and the callback payload. For
 instance:
 
-invalidatecursor:
+invalidatecursor: <payload>
+
+The payload contains a rectangle describing the cursor position.
 
 invalidatetiles: <payload>
 
commit 6f9629258cd595dcb4029be9264b28a04d0d30ea
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Fri Sep 2 01:19:33 2016 +0530

    loleaflet: Fix inconsistent user count for views going active
    
    Iterating viewCursor object won't give correct number of live
    viewids because inactive views don't receive any viewcursor
    messages.
    
    Rather use map._viewInfo object because as of now, we do receive
    these 'addview' message for inactive views, so this place should
    always remain updated and prevent giving inconsistent view counts
    across different clients.
    
    Change-Id: If210049e76b5f2b91371a21863f6019cbccdb9ca

diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index a2d6850..388582f 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -699,7 +699,7 @@ L.TileLayer = L.GridLayer.extend({
 	},
 
 	_onRemAllViewMsg: function(textMsg) {
-		for (var viewId in this._viewCursors) {
+		for (var viewId in this._map._viewInfo) {
 			this._onRemViewMsg('remview: ' + viewId);
 		}
 	},


More information about the Libreoffice-commits mailing list