common/MessageQueue.cpp |  179 +++++++++++++++++++++++++++++++++++++++++-------
 common/MessageQueue.hpp |   37 +++++++++
 common/Protocol.hpp     |    7 +
 kit/Kit.cpp             |   10 +-
 test/TileQueueTests.cpp |    8 +-
 5 files changed, 208 insertions(+), 33 deletions(-)

New commits:
commit c326228774e720011b8845f0ee14b4c0a5f1d4f2
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Tue Nov 29 21:55:44 2016 -0500

    loolwsd: improved MessageQueue
    
    Tiles no longer hog the queue ahead of all else.
    
    We now give priority to callback events, so clients
    get to know the document state sooner.
    
    Since tiles take long to render, an equal time
    is given to non-tiles (capped at 100ms).
    
    Finally, Impress preview tiles are given
    the lowest priority and rendered only when
    the queue is drained.
    
    Change-Id: I922c1e11200e5675f50d86b83baee1588cbbf66f
    Reviewed-on: https://gerrit.libreoffice.org/31394
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index d9696e1..d6e1eed 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -191,46 +191,174 @@ int TileQueue::priority(const std::string& tileMsg)
     return -1;
 }
 
-void TileQueue::deprioritizePreviews()
+int TileQueue::findFirstNonPreview(bool preferTiles) const
 {
+    int firstTile = -1;
+    int firstElse = -1;
     for (size_t i = 0; i < _queue.size(); ++i)
     {
-        const auto front = _queue.front();
-        const std::string message(front.data(), front.size());
+        const auto& front = _queue[i];
+        const bool isTile = LOOLProtocol::matchPrefix("tile", front);
+        //LOG_WRN("#" << i << " " << (isTile ? "isTile" : "non-tile"));
 
-        // stop at the first non-tile or non-'id' (preview) message
-        std::string id;
-        if (!LOOLProtocol::matchPrefix("tile", message) ||
-            !LOOLProtocol::getTokenStringFromMessage(message, "id", id))
+        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)
         {
             break;
         }
+    }
 
-        _queue.erase(_queue.begin());
-        _queue.push_back(front);
+    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;
+    }
+
+    // We can still do some more non-tiles.
+    LOG_TRC("Have time for more non-tiles.");
+    return false;
+}
+
 MessageQueue::Payload TileQueue::get_impl()
 {
-    const auto front = _queue.front();
+    LOG_TRC("MessageQueue depth: " << _queue.size());
 
-    auto msg = std::string(front.data(), front.size());
+    auto front = _queue.front();
+    bool isTileFirst = LOOLProtocol::matchPrefix("tile", front);
+    //LOG_WRN("isTileFirst: " << isTileFirst);
 
-    std::string id;
-    bool isTile = LOOLProtocol::matchPrefix("tile", msg);
-    bool isPreview = isTile && LOOLProtocol::getTokenStringFromMessage(msg, 
"id", id);
-    if (!isTile || isPreview)
+    if (_queue.size() == 1)
     {
-        // Don't combine non-tiles or tiles with id.
-        LOG_TRC("MessageQueue res: " << msg);
+        updateTimestamps(isTileFirst);
+
+        //const auto msg = std::string(front.data(), front.size());
+        //LOG_TRC("MessageQueue res only: " << msg);
         _queue.erase(_queue.begin());
+        return front;
+    }
 
-        // de-prioritize the other tiles with id - usually the previews in
-        // Impress
-        if (isPreview)
-            deprioritizePreviews();
+    // Drain callbacks as soon and as fast as possible.
+    if (!isTileFirst && LOOLProtocol::matchPrefix("callback", front))
+    {
+        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);
+        _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;
     }
 
@@ -238,6 +366,7 @@ 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];
@@ -246,6 +375,7 @@ 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))
         {
@@ -277,6 +407,7 @@ 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))
         {
@@ -286,7 +417,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))
@@ -300,6 +431,8 @@ MessageQueue::Payload TileQueue::get_impl()
         }
     }
 
+    updateTimestamps(true);
+
     LOG_TRC("Combined " << tiles.size() << " tiles, leaving " << _queue.size() 
<< " in queue.");
 
     if (tiles.size() == 1)
@@ -309,7 +442,7 @@ MessageQueue::Payload TileQueue::get_impl()
         return Payload(msg.data(), msg.data() + msg.size());
     }
 
-    auto tileCombined = TileCombined::create(tiles).serialize("tilecombine");
+    const 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 3cd023b..afea17f 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -85,6 +85,14 @@ 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 });
@@ -129,9 +137,25 @@ private:
     /// Search the queue for a duplicate tile and remove it (if present).
     void removeDuplicate(const std::string& tileMsg);
 
-    /// De-prioritize the previews (tiles with 'id') - move them to the end of
-    /// the queue.
-    void deprioritizePreviews();
+    /// 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);
 
     /// Priority of the given tile message.
     /// -1 means the lowest prio (the tile does not intersect any of the 
cursors),
@@ -144,6 +168,13 @@ 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 e95090d..da64ee3 100644
--- a/common/Protocol.hpp
+++ b/common/Protocol.hpp
@@ -111,6 +111,13 @@ 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 28df0d8..e5ddd37 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -607,6 +607,7 @@ 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:");
@@ -637,7 +638,8 @@ public:
             return;
         }
 
-        const double area = tile.getWidth() * tile.getHeight();
+        LOG_TRC("+paintTile at (" << tile.getPart() << ',' << 
tile.getTilePosX() << ',' << tile.getTilePosY() <<
+                ") " << "ver: " << tile.getVersion());
         Timestamp timestamp;
         _loKitDocument->paintPartTile(pixmap.data(), tile.getPart(),
                                       tile.getWidth(), tile.getHeight(),
@@ -692,7 +694,8 @@ public:
         const size_t tilesByY = renderArea.getHeight() / 
tileCombined.getTileHeight();
         const size_t pixmapWidth = tilesByX * tileCombined.getWidth();
         const size_t pixmapHeight = tilesByY * tileCombined.getHeight();
-        const size_t pixmapSize = 4 * pixmapWidth * pixmapHeight;
+        const double area = pixmapWidth * pixmapHeight;
+        const size_t pixmapSize = 4 * area;
         std::vector<unsigned char> pixmap(pixmapSize, 0);
 
         if (!_loKitDocument)
@@ -708,7 +711,8 @@ public:
             return;
         }
 
-        const double area = pixmapWidth * pixmapHeight;
+        LOG_DBG("+paintTile (combined) at (" << renderArea.getLeft() << ", " 
<< renderArea.getTop() << "), (" <<
+                renderArea.getWidth() << ", " << renderArea.getHeight() << ") 
ver: " << tileCombined.getVersion());
         Timestamp timestamp;
         _loKitDocument->paintPartTile(pixmap.data(), tileCombined.getPart(),
                                       pixmapWidth, pixmapHeight,
diff --git a/test/TileQueueTests.cpp b/test/TileQueueTests.cpp
index dea692b..868d78e 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";
+    const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 
tileposy=0 tilewidth=3840 tileheight=3840 ver=-1";
     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";
+    const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 
tileposy=253440 tilewidth=3840 tileheight=3840 ver=-1";
     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(previews[0], payloadAsString(queue.get()));
     CPPUNIT_ASSERT_EQUAL(tiles[0], payloadAsString(queue.get()));
+    CPPUNIT_ASSERT_EQUAL(previews[0], payloadAsString(queue.get()));
     CPPUNIT_ASSERT_EQUAL(previews[1], payloadAsString(queue.get()));
 
     queue.put(tiles[1]);
 
-    CPPUNIT_ASSERT_EQUAL(previews[2], payloadAsString(queue.get()));
     CPPUNIT_ASSERT_EQUAL(tiles[1], payloadAsString(queue.get()));
+    CPPUNIT_ASSERT_EQUAL(previews[2], payloadAsString(queue.get()));
     CPPUNIT_ASSERT_EQUAL(previews[3], payloadAsString(queue.get()));
 
     // stays empty after all is done
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to