[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
Tue Sep 20 03:31:02 UTC 2016
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.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.
Reviewed-on: https://gerrit.libreoffice.org/28575
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 59eaacd2f87f46c4c4d2963ef54f4d20d346b2d0)
Change-Id: I77813267a95a724491165792ec020ae00953c05e
Reviewed-on: https://gerrit.libreoffice.org/29066
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 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 \
More information about the Libreoffice-commits
mailing list