[Libreoffice-commits] online.git: loolwsd/ClientSession.cpp loolwsd/ClientSession.hpp loolwsd/DocumentBroker.cpp loolwsd/LOOLWSD.cpp loolwsd/test loolwsd/TileCache.cpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Thu Sep 1 03:59:49 UTC 2016


 loolwsd/ClientSession.cpp  |    2 --
 loolwsd/ClientSession.hpp  |    9 ---------
 loolwsd/DocumentBroker.cpp |    7 ++++---
 loolwsd/LOOLWSD.cpp        |   28 ++++++----------------------
 loolwsd/TileCache.cpp      |   27 ++++++++++++++++++++-------
 loolwsd/test/Makefile.am   |    1 +
 6 files changed, 31 insertions(+), 43 deletions(-)

New commits:
commit 59eaacd2f87f46c4c4d2963ef54f4d20d346b2d0
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Wed Aug 31 23:34:41 2016 -0400

    loolwsd: remove tile queue and simplify tile response
    
    Tile queue was used to process canceltiles commands.
    Since those are handled by TileCache, there is no need
    for queues and the threads that pump them.
    
    But because these queues were also used to buffer between
    WSD internals and clients, such that a slow client wouldn't
    block WSD while sending back tiles, it is necessary
    to reword that logic.
    In subsequent commits that will change as well.
    
    With this change not only do we save a thread per client,
    but we also reduce latency of tile, and improve typing
    responsiveness, by almost 3x or more! Latencies are
    down to ~15ms from almost 50ms.
    
    Change-Id: I9bb5856efed28caea9d4e6f94f77b093779e5241
    Reviewed-on: https://gerrit.libreoffice.org/28575
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 40deeee..c61c574 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -37,11 +37,9 @@ using Poco::StringTokenizer;
 ClientSession::ClientSession(const std::string& id,
                              std::shared_ptr<Poco::Net::WebSocket> ws,
                              std::shared_ptr<DocumentBroker> docBroker,
-                             std::shared_ptr<BasicTileQueue> queue,
                              bool readOnly) :
     LOOLSession(id, Kind::ToClient, ws),
     _docBroker(std::move(docBroker)),
-    _queue(std::move(queue)),
     _haveEditLock(std::getenv("LOK_VIEW_CALLBACK")),
     _isReadOnly(readOnly),
     _loadFailed(false),
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index dc531c8..3a9b1a1 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -23,7 +23,6 @@ public:
     ClientSession(const std::string& id,
                   std::shared_ptr<Poco::Net::WebSocket> ws,
                   std::shared_ptr<DocumentBroker> docBroker,
-                  std::shared_ptr<BasicTileQueue> queue,
                   bool isReadOnly = false);
 
     virtual ~ClientSession();
@@ -60,11 +59,6 @@ public:
         _loadFailed = true;
     }
 
-    void sendToInputQueue(const std::string& message)
-    {
-        _queue->put(message);
-    }
-
     std::shared_ptr<DocumentBroker> getDocumentBroker() const { return _docBroker; }
 
 private:
@@ -85,9 +79,6 @@ private:
 
     std::shared_ptr<DocumentBroker> _docBroker;
 
-    /// The incoming message queue.
-    std::shared_ptr<BasicTileQueue> _queue;
-
     // If this document holds the edit lock.
     // An edit lock will only allow the current session to make edits,
     // while other session opening the same document can only see
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 1e82696..afa8460 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -317,7 +317,6 @@ bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
             _lastFileModifiedTime.fromEpochTime(0);
 
             // We do not want save to terminate editing mode if we are in edit mode now
-
             std::ostringstream oss;
             // arguments init
             oss << "{";
@@ -343,8 +342,10 @@ bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
             // arguments end
             oss << "}";
 
-            Log::debug(".uno:Save arguments: " + oss.str());
-            sessionIt.second->sendToInputQueue("uno .uno:Save " + oss.str());
+            const auto saveArgs = oss.str();
+            Log::trace(".uno:Save arguments: " + saveArgs);
+            const auto command = "uno .uno:Save " + saveArgs;
+            sessionIt.second->handleInput(command.data(), command.size());
             return true;
         }
     }
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 4d1628b..5efbd39 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -399,7 +399,7 @@ private:
 
                     // Load the document.
                     std::shared_ptr<WebSocket> ws;
-                    auto session = std::make_shared<ClientSession>(id, ws, docBroker, nullptr);
+                    auto session = std::make_shared<ClientSession>(id, ws, docBroker);
 
                     // Request the child to connect to us and add this session.
                     auto sessionsCount = docBroker->addSession(session);
@@ -662,10 +662,7 @@ private:
         std::shared_ptr<ClientSession> session;
         try
         {
-            // For ToClient sessions, we store incoming messages in a queue and have a separate
-            // thread to pump them. This is to empty the queue when we get a "canceltiles" message.
-            auto queue = std::make_shared<BasicTileQueue>();
-            session = std::make_shared<ClientSession>(id, ws, docBroker, queue, isReadOnly);
+            session = std::make_shared<ClientSession>(id, ws, docBroker, isReadOnly);
             if (!fileinfo._userName.empty())
             {
                 Log::debug(uriPublic.toString() + " requested with username [" + fileinfo._userName + "]");
@@ -692,18 +689,13 @@ private:
             ws->sendFrame(status.data(), (int) status.size());
 
             // Let messages flow
-            QueueHandler handler(queue, session, "wsd_queue_" + session->getId());
-            Thread queueHandlerThread;
-            queueHandlerThread.start(handler);
-
             IoUtil::SocketProcessor(ws,
-                [&queue](const std::vector<char>& payload)
+                [&session](const std::vector<char>& payload)
                 {
-                    queue->put(payload);
-                    return true;
+                    return session->handleInput(payload.data(), payload.size());
                 },
                 [&session]() { session->closeFrame(); },
-                [&queueHandlerThread]() { return TerminationFlag || !queueHandlerThread.isRunning(); });
+                []() { return !!TerminationFlag; });
 
             {
                 std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
@@ -743,12 +735,6 @@ private:
                 }
             }
 
-            if (session->isLoadFailed())
-            {
-                Log::info("Clearing the queue.");
-                queue->clear();
-            }
-
             if (sessionsCount == 0)
             {
                 std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
@@ -762,9 +748,7 @@ private:
             }
 
             LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "EndSession: " + uri);
-            Log::info("Finishing GET request handler for session [" + id + "]. Joining the queue.");
-            queue->put("eof");
-            queueHandlerThread.join();
+            Log::info("Finishing GET request handler for session [" + id + "].");
         }
         catch (const std::exception& exc)
         {
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index 8f05a14..ca6af9b 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -166,19 +166,32 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
     {
         if (!tileBeingRendered->_subscribers.empty())
         {
-            const std::string message = tile.serialize("tile");
-            Log::debug("Sending tile message to subscribers: " + message);
+            std::string response = tile.serialize("tile:");
+            Log::debug("Sending tile message to subscribers: " + response);
+            response += '\n';
+
+            std::vector<char> output;
+            output.reserve(static_cast<size_t>(4) * tile.getWidth() * tile.getHeight());
+            output.resize(response.size());
+            std::memcpy(output.data(), response.data(), response.size());
+
+            const auto pos = output.size();
+            output.resize(pos + size);
+            std::memcpy(output.data() + pos, data, size);
 
             for (const auto& i: tileBeingRendered->_subscribers)
             {
                 auto subscriber = i.lock();
                 if (subscriber)
                 {
-                    //FIXME: This is inefficient; should just send directly to each client (although that is risky as well!
-                    // Re-emit the tile command in the other thread(s) to re-check and hit
-                    // the cache. Construct the message from scratch to contain only the
-                    // mandatory parts of the message.
-                    subscriber->sendToInputQueue(message);
+                    try
+                    {
+                        subscriber->sendBinaryFrame(output.data(), output.size());
+                    }
+                    catch (const std::exception& ex)
+                    {
+                        Log::warn("Failed to send tile to " + subscriber->getName() + ": " + ex.what());
+                    }
                 }
             }
         }
diff --git a/loolwsd/test/Makefile.am b/loolwsd/test/Makefile.am
index 9da80cd..2723dc2 100644
--- a/loolwsd/test/Makefile.am
+++ b/loolwsd/test/Makefile.am
@@ -20,6 +20,7 @@ wsd_sources = \
             ../IoUtil.cpp \
             ../Log.cpp \
             ../LOOLProtocol.cpp \
+            ../LOOLSession.cpp \
             ../TileCache.cpp \
             ../MessageQueue.cpp \
             ../Unit.cpp \


More information about the Libreoffice-commits mailing list