[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-0' - common/MessageQueue.cpp common/MessageQueue.hpp common/Protocol.hpp kit/Kit.cpp test/TileQueueTests.cpp
Jan Holesovsky
kendy at collabora.com
Fri Dec 2 11:00:58 UTC 2016
common/MessageQueue.cpp | 179 ++++++------------------------------------------
common/MessageQueue.hpp | 37 ---------
common/Protocol.hpp | 7 -
kit/Kit.cpp | 10 --
test/TileQueueTests.cpp | 8 +-
5 files changed, 33 insertions(+), 208 deletions(-)
New commits:
commit 09668f9040ba7575b57ecf0106c6bfc6df2a2b17
Author: Jan Holesovsky <kendy at collabora.com>
Date: Fri Dec 2 10:58:48 2016 +0100
Revert "loolwsd: improved MessageQueue"
This reverts commit ff74d75d64e7ddf6ef7f4b269343fbb398992b82.
Change-Id: I71e657ad6069e138446a15d6d30c6df5999c7f12
Reviewed-on: https://gerrit.libreoffice.org/31537
Reviewed-by: Tor Lillqvist <tml at collabora.com>
Tested-by: Tor Lillqvist <tml at collabora.com>
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index d6e1eed..d9696e1 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -191,174 +191,46 @@ int TileQueue::priority(const std::string& tileMsg)
return -1;
}
-int TileQueue::findFirstNonPreview(bool preferTiles) const
+void TileQueue::deprioritizePreviews()
{
- int firstTile = -1;
- int firstElse = -1;
for (size_t i = 0; i < _queue.size(); ++i)
{
- const auto& front = _queue[i];
- const bool isTile = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("#" << i << " " << (isTile ? "isTile" : "non-tile"));
+ const auto front = _queue.front();
+ const std::string message(front.data(), front.size());
- if (isTile && firstTile < 0)
- {
- const std::string msg(front.data(), front.size());
- std::string id;
- const bool isPreview = LOOLProtocol::getTokenStringFromMessage(msg, "id", id);
- //LOG_WRN("#" << i << " " << (isPreview ? "isPreview" : "isTile") << ": " << msg);
- if (!isPreview)
- {
- firstTile = i;
- //LOG_WRN("firstTile: #" << i);
- }
- }
- else if (!isTile && firstElse < 0)
- {
- firstElse = i;
- //LOG_WRN("firstElse: #" << i);
- }
- else if (firstTile >=0 && firstElse >= 0)
+ // stop at the first non-tile or non-'id' (preview) message
+ std::string id;
+ if (!LOOLProtocol::matchPrefix("tile", message) ||
+ !LOOLProtocol::getTokenStringFromMessage(message, "id", id))
{
break;
}
- }
-
- if (preferTiles && firstTile >= 0)
- {
- return firstTile;
- }
-
- if (firstElse >= 0)
- {
- return firstElse;
- }
-
- if (firstTile >= 0)
- {
- return firstTile;
- }
- return -1;
-}
-
-void TileQueue::bumpToTop(const size_t index)
-{
- if (index > 0)
- {
- Payload payload(_queue[index]);
- //LOG_WRN("Bumping: " << std::string(payload.data(), payload.size()));
-
- _queue.erase(_queue.begin() + index);
- _queue.insert(_queue.begin(), payload);
- }
-}
-
-void TileQueue::updateTimestamps(const bool isTile)
-{
- if (isTile)
- {
- _lastGetTile = true;
- _lastTileGetTime = std::chrono::steady_clock::now();
- }
- else if (_lastGetTile)
- {
- // Update non-tile timestamp when switching from tiles.
- _lastGetTile = false;
- _lastGetTime = std::chrono::steady_clock::now();
- }
-}
-
-bool TileQueue::shouldPreferTiles() const
-{
- if (_lastGetTile)
- {
- // If we had just done a tile, do something else.
- LOG_TRC("Last was tile, doing non-tiles.");
- return false;
- }
-
- // Check how long it's been since we'd done tiles.
- const auto tileDuration = (_lastGetTime - _lastTileGetTime);
- const auto tileDurationMs = std::chrono::duration_cast<std::chrono::milliseconds>(tileDuration).count();
- const auto duration = (std::chrono::steady_clock::now() - _lastGetTime);
- const auto durationMs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();
- LOG_TRC("Tile duration: " << tileDurationMs << "ms, nonTile duration: " << durationMs << "ms.");
-
- if (durationMs > MaxTileSkipDurationMs)
- {
- LOG_TRC("Capping non-tiles to 100ms. Prefer tiles now.");
- return true;
- }
-
- if (durationMs > tileDurationMs)
- {
- LOG_TRC("Capping non-tiles to tileDurationMs (" << tileDurationMs << "). Prefer tiles now.");
- return true;
+ _queue.erase(_queue.begin());
+ _queue.push_back(front);
}
-
- // We can still do some more non-tiles.
- LOG_TRC("Have time for more non-tiles.");
- return false;
}
MessageQueue::Payload TileQueue::get_impl()
{
- LOG_TRC("MessageQueue depth: " << _queue.size());
-
- auto front = _queue.front();
- bool isTileFirst = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("isTileFirst: " << isTileFirst);
-
- if (_queue.size() == 1)
- {
- updateTimestamps(isTileFirst);
+ const auto front = _queue.front();
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res only: " << msg);
- _queue.erase(_queue.begin());
- return front;
- }
+ auto msg = std::string(front.data(), front.size());
- // Drain callbacks as soon and as fast as possible.
- if (!isTileFirst && LOOLProtocol::matchPrefix("callback", front))
+ std::string id;
+ bool isTile = LOOLProtocol::matchPrefix("tile", msg);
+ bool isPreview = isTile && LOOLProtocol::getTokenStringFromMessage(msg, "id", id);
+ if (!isTile || isPreview)
{
- updateTimestamps(false);
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res call: " << msg);
- _queue.erase(_queue.begin());
- return front;
- }
-
- // TODO: Try draining all callbacks first.
-
- const bool preferTiles = shouldPreferTiles();
- const int nonPreviewIndex = findFirstNonPreview(preferTiles);
- //LOG_WRN("First non-preview: " << nonPreviewIndex);
- if (nonPreviewIndex < 0)
- {
- // We are left with previews only.
- updateTimestamps(true); // We're doing a tile.
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res prev: " << msg);
+ // Don't combine non-tiles or tiles with id.
+ LOG_TRC("MessageQueue res: " << msg);
_queue.erase(_queue.begin());
- return front;
- }
- bumpToTop(nonPreviewIndex);
- front = _queue.front();
- isTileFirst = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("New front: " << std::string(front.data(), front.size()));
-
- if (!isTileFirst)
- {
- updateTimestamps(false);
+ // de-prioritize the other tiles with id - usually the previews in
+ // Impress
+ if (isPreview)
+ deprioritizePreviews();
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res call: " << msg);
- _queue.erase(_queue.begin());
return front;
}
@@ -366,7 +238,6 @@ MessageQueue::Payload TileQueue::get_impl()
// position, otherwise handle the one that is at the front
int prioritized = 0;
int prioritySoFar = -1;
- std::string msg(front.data(), front.size());
for (size_t i = 0; i < _queue.size(); ++i)
{
auto& it = _queue[i];
@@ -375,7 +246,6 @@ MessageQueue::Payload TileQueue::get_impl()
// avoid starving - stop the search when we reach a non-tile,
// otherwise we may keep growing the queue of unhandled stuff (both
// tiles and non-tiles)
- std::string id;
if (!LOOLProtocol::matchPrefix("tile", prio) ||
LOOLProtocol::getTokenStringFromMessage(prio, "id", id))
{
@@ -407,7 +277,6 @@ MessageQueue::Payload TileQueue::get_impl()
{
auto& it = _queue[i];
msg = std::string(it.data(), it.size());
- std::string id;
if (!LOOLProtocol::matchPrefix("tile", msg) ||
LOOLProtocol::getTokenStringFromMessage(msg, "id", id))
{
@@ -417,7 +286,7 @@ MessageQueue::Payload TileQueue::get_impl()
}
auto tile2 = TileDesc::parse(msg);
- //LOG_TRC("Combining candidate: " << msg);
+ LOG_TRC("Combining candidate: " << msg);
// Check if it's on the same row.
if (tiles[0].onSameRow(tile2))
@@ -431,8 +300,6 @@ MessageQueue::Payload TileQueue::get_impl()
}
}
- updateTimestamps(true);
-
LOG_TRC("Combined " << tiles.size() << " tiles, leaving " << _queue.size() << " in queue.");
if (tiles.size() == 1)
@@ -442,7 +309,7 @@ MessageQueue::Payload TileQueue::get_impl()
return Payload(msg.data(), msg.data() + msg.size());
}
- const auto tileCombined = TileCombined::create(tiles).serialize("tilecombine");
+ auto tileCombined = TileCombined::create(tiles).serialize("tilecombine");
LOG_TRC("MessageQueue res: " << tileCombined);
return Payload(tileCombined.data(), tileCombined.data() + tileCombined.size());
}
diff --git a/common/MessageQueue.hpp b/common/MessageQueue.hpp
index afea17f..3cd023b 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -85,14 +85,6 @@ private:
};
public:
-
- TileQueue() :
- _lastTileGetTime(std::chrono::steady_clock::now()),
- _lastGetTime(_lastTileGetTime),
- _lastGetTile(true)
- {
- }
-
void updateCursorPosition(int viewId, int part, int x, int y, int width, int height)
{
auto cursorPosition = CursorPosition({ part, x, y, width, height });
@@ -137,25 +129,9 @@ private:
/// Search the queue for a duplicate tile and remove it (if present).
void removeDuplicate(const std::string& tileMsg);
- /// Find the index of the first non-preview entry.
- /// When preferTiles is false, it'll return index of
- /// the first non-tile, otherwise, the index of the
- /// first tile is returned.
- /// Returns -1 if only previews are left.
- int findFirstNonPreview(bool preferTiles) const;
-
- /// Returns true if we should try to return
- /// a tile, otherwise a non-tile.
- bool shouldPreferTiles() const;
-
- /// Update the tile/non-tile timestamps to
- /// track how much time we spend for each.
- /// isTile marks if the current message
- /// is a tile or not.
- void updateTimestamps(const bool isTile);
-
- /// Given a positive index, move it to the top.
- void bumpToTop(const size_t index);
+ /// De-prioritize the previews (tiles with 'id') - move them to the end of
+ /// the queue.
+ void deprioritizePreviews();
/// Priority of the given tile message.
/// -1 means the lowest prio (the tile does not intersect any of the cursors),
@@ -168,13 +144,6 @@ private:
/// Check the views in the order of how the editing (cursor movement) has
/// been happening (0 == oldest, size() - 1 == newest).
std::vector<int> _viewOrder;
-
- std::chrono::steady_clock::time_point _lastTileGetTime;
- std::chrono::steady_clock::time_point _lastGetTime;
- bool _lastGetTile;
-
- /// For responsiveness, we shouldn't have higher latency.
- static const int MaxTileSkipDurationMs = 100;
};
#endif
diff --git a/common/Protocol.hpp b/common/Protocol.hpp
index da64ee3..e95090d 100644
--- a/common/Protocol.hpp
+++ b/common/Protocol.hpp
@@ -111,13 +111,6 @@ namespace LOOLProtocol
}
inline
- bool matchPrefix(const std::string& prefix, const std::vector<char>& message)
- {
- return (message.size() >= prefix.size() &&
- prefix.compare(0, prefix.size(), message.data(), prefix.size()) == 0);
- }
-
- inline
bool matchPrefix(const std::string& prefix, const std::string& message, const bool ignoreWhitespace)
{
if (ignoreWhitespace)
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index e5ddd37..28df0d8 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -607,7 +607,6 @@ public:
{
assert(ws && "Expected a non-null websocket.");
auto tile = TileDesc::parse(tokens);
- const double area = tile.getWidth() * tile.getHeight();
// Send back the request with all optional parameters given in the request.
const auto tileMsg = tile.serialize("tile:");
@@ -638,8 +637,7 @@ public:
return;
}
- LOG_TRC("+paintTile at (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() <<
- ") " << "ver: " << tile.getVersion());
+ const double area = tile.getWidth() * tile.getHeight();
Timestamp timestamp;
_loKitDocument->paintPartTile(pixmap.data(), tile.getPart(),
tile.getWidth(), tile.getHeight(),
@@ -694,8 +692,7 @@ public:
const size_t tilesByY = renderArea.getHeight() / tileCombined.getTileHeight();
const size_t pixmapWidth = tilesByX * tileCombined.getWidth();
const size_t pixmapHeight = tilesByY * tileCombined.getHeight();
- const double area = pixmapWidth * pixmapHeight;
- const size_t pixmapSize = 4 * area;
+ const size_t pixmapSize = 4 * pixmapWidth * pixmapHeight;
std::vector<unsigned char> pixmap(pixmapSize, 0);
if (!_loKitDocument)
@@ -711,8 +708,7 @@ public:
return;
}
- LOG_DBG("+paintTile (combined) at (" << renderArea.getLeft() << ", " << renderArea.getTop() << "), (" <<
- renderArea.getWidth() << ", " << renderArea.getHeight() << ") ver: " << tileCombined.getVersion());
+ const double area = pixmapWidth * pixmapHeight;
Timestamp timestamp;
_loKitDocument->paintPartTile(pixmap.data(), tileCombined.getPart(),
pixmapWidth, pixmapHeight,
diff --git a/test/TileQueueTests.cpp b/test/TileQueueTests.cpp
index 868d78e..dea692b 100644
--- a/test/TileQueueTests.cpp
+++ b/test/TileQueueTests.cpp
@@ -56,10 +56,10 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
void TileQueueTests::testTileQueuePriority()
{
- const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 ver=-1";
+ const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840";
const std::string resHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 ver=-1";
const TileQueue::Payload payloadHigh(resHigh.data(), resHigh.data() + resHigh.size());
- const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 ver=-1";
+ const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840";
const std::string resLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 ver=-1";
const TileQueue::Payload payloadLow(resLow.data(), resLow.data() + resLow.size());
@@ -232,14 +232,14 @@ void TileQueueTests::testPreviewsDeprioritization()
queue.put(tiles[0]);
- CPPUNIT_ASSERT_EQUAL(tiles[0], payloadAsString(queue.get()));
CPPUNIT_ASSERT_EQUAL(previews[0], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(tiles[0], payloadAsString(queue.get()));
CPPUNIT_ASSERT_EQUAL(previews[1], payloadAsString(queue.get()));
queue.put(tiles[1]);
- CPPUNIT_ASSERT_EQUAL(tiles[1], payloadAsString(queue.get()));
CPPUNIT_ASSERT_EQUAL(previews[2], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(tiles[1], payloadAsString(queue.get()));
CPPUNIT_ASSERT_EQUAL(previews[3], payloadAsString(queue.get()));
// stays empty after all is done
More information about the Libreoffice-commits
mailing list