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

New commits:
commit e7272019dc28da7b66a7db145bda9a9c3db1380c
Author: Ashod Nakashian <ashod.nakash...@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.
    
    Reviewed-on: https://gerrit.libreoffice.org/28575
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>
    (cherry picked from commit 59eaacd2f87f46c4c4d2963ef54f4d20d346b2d0)
    
    Change-Id: I77813267a95a724491165792ec020ae00953c05e
    Reviewed-on: https://gerrit.libreoffice.org/29066
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 533844f..b493c58 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)),
     _isReadOnly(readOnly),
     _loadFailed(false),
     _loadPart(-1)
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index e6f3098..a285efe 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();
@@ -58,11 +57,6 @@ public:
         _loadFailed = true;
     }
 
-    void sendToInputQueue(const std::string& message)
-    {
-        _queue->put(message);
-    }
-
     std::shared_ptr<DocumentBroker> getDocumentBroker() const { return 
_docBroker; }
 
 private:
@@ -83,9 +77,6 @@ private:
 
     std::shared_ptr<DocumentBroker> _docBroker;
 
-    /// The incoming message queue.
-    std::shared_ptr<BasicTileQueue> _queue;
-
     // Whether the session is opened as readonly
     bool _isReadOnly;
 
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 1e58e76..78cc9e7 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -364,8 +364,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 9f1d6d7..266ac7c 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);
@@ -686,10 +686,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 + "]");
@@ -716,18 +713,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);
@@ -767,12 +759,6 @@ private:
                 }
             }
 
-            if (session->isLoadFailed())
-            {
-                Log::info("Clearing the queue.");
-                queue->clear();
-            }
-
             if (sessionsCount == 0)
             {
                 std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
@@ -786,9 +772,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 b3cf0ea..9e3bca7 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -168,19 +168,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 843bdff..ab0918c 100644
--- a/loolwsd/test/Makefile.am
+++ b/loolwsd/test/Makefile.am
@@ -21,6 +21,7 @@ wsd_sources = \
             ../IoUtil.cpp \
             ../Log.cpp \
             ../LOOLProtocol.cpp \
+            ../LOOLSession.cpp \
             ../TileCache.cpp \
             ../MessageQueue.cpp \
             ../Unit.cpp \
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to