[Libreoffice-commits] online.git: Branch 'feature/deduplication' - common/MessageQueue.cpp common/MessageQueue.hpp kit/Kit.cpp test/TileQueueTests.cpp
Jan Holesovsky
kendy at collabora.com
Wed Jan 25 18:21:53 UTC 2017
common/MessageQueue.cpp | 179 ++++++------------------------------------------
common/MessageQueue.hpp | 37 ---------
kit/Kit.cpp | 10 --
test/TileQueueTests.cpp | 8 +-
4 files changed, 33 insertions(+), 201 deletions(-)
New commits:
commit e35882318fc6f2b8471d6b9feca59a5183f2defd
Author: Jan Holesovsky <kendy at collabora.com>
Date: Wed Jan 25 19:14:38 2017 +0100
Revert "loolwsd: improved MessageQueue"
Now we don't get a situation where there would be a tremendous amount of
invalidates & tile render requests piled in the queue, so we can do it
deterministic again.
The only thing that could potentially pile in the queue are the keypresses
events sent from the clients, but that is a different problem anyway.
This reverts commit c326228774e720011b8845f0ee14b4c0a5f1d4f2.
Change-Id: I98e199eab0187bf5f47ce322ac1b1b2e3b976b85
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index f31d665..54f1871 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -401,174 +401,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;
}
TileQueue::Payload TileQueue::get_impl()
{
- LOG_TRC("MessageQueue depth: " << _queue.size());
+ const auto front = _queue.front();
- auto front = _queue.front();
- bool isTileFirst = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("isTileFirst: " << isTileFirst);
-
- if (_queue.size() == 1)
- {
- updateTimestamps(isTileFirst);
-
- //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);
+ // Don't combine non-tiles or tiles with id.
+ LOG_TRC("MessageQueue res: " << msg);
_queue.erase(_queue.begin());
- return front;
- }
- // TODO: Try draining all callbacks first.
+ // de-prioritize the other tiles with id - usually the previews in
+ // Impress
+ if (isPreview)
+ deprioritizePreviews();
- 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);
- _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);
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res call: " << msg);
- _queue.erase(_queue.begin());
return front;
}
@@ -576,7 +448,6 @@ TileQueue::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];
@@ -585,7 +456,6 @@ TileQueue::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))
{
@@ -617,7 +487,6 @@ TileQueue::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))
{
@@ -627,7 +496,7 @@ TileQueue::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))
@@ -641,8 +510,6 @@ TileQueue::Payload TileQueue::get_impl()
}
}
- updateTimestamps(true);
-
LOG_TRC("Combined " << tiles.size() << " tiles, leaving " << _queue.size() << " in queue.");
if (tiles.size() == 1)
@@ -652,7 +519,7 @@ TileQueue::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 1138259..2ab30fb 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -147,14 +147,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)
{
const auto cursorPosition = CursorPosition({ part, x, y, width, height });
@@ -212,25 +204,9 @@ private:
/// @return New message to put into the queue. If empty, use what was in callbackMsg.
std::string removeCallbackDuplicate(const std::string& callbackMsg);
- /// 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),
@@ -243,13 +219,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/kit/Kit.cpp b/kit/Kit.cpp
index 30966f9..16e20ac 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -588,7 +588,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:");
@@ -619,8 +618,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(),
@@ -683,8 +681,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);
std::unique_lock<std::mutex> lock(_documentMutex);
@@ -700,8 +697,7 @@ public:
return;
}
- LOG_DBG("+paintTile (combined) at (" << renderArea.getLeft() << ", " << renderArea.getTop() << "), (" <<
- renderArea.getWidth() << ", " << renderArea.getHeight() << ").");
+ 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 884ee51..8c4a5ad 100644
--- a/test/TileQueueTests.cpp
+++ b/test/TileQueueTests.cpp
@@ -66,10 +66,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 oldhash=0 hash=0 ver=-1";
+ const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0";
const std::string resHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 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 oldhash=0 hash=0 ver=-1";
+ const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0";
const std::string resLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
const TileQueue::Payload payloadLow(resLow.data(), resLow.data() + resLow.size());
@@ -242,14 +242,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