[Libreoffice-commits] online.git: Branch 'feature/latency' - 4 commits - wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/TileCache.cpp wsd/TileCache.hpp

Tamás Zolnai tamas.zolnai at collabora.com
Thu Jul 5 11:46:41 UTC 2018


 wsd/ClientSession.cpp  |   44 ++++++++------------------------------------
 wsd/ClientSession.hpp  |   16 +++++++---------
 wsd/DocumentBroker.cpp |   33 +++++++++++++++++----------------
 wsd/TileCache.cpp      |    5 +++++
 wsd/TileCache.hpp      |    3 +++
 5 files changed, 40 insertions(+), 61 deletions(-)

New commits:
commit 769359d27f078ab9c57b504db5a625fb8368a1f7
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
Date:   Thu Jun 28 18:20:14 2018 +0200

    Use an upper limit for number of tiles we push to the network
    
    I used number 25 as this limit. It's an approximate value. It's enough
    to handle the first 5-10 character input without waiting for tileprocessed
    messages. After the first 5-10 characters the tileprocessed messages are
    arriving continuously (same frequency as the typing speed), so for the later
    character inputs we always have some tileprocessed messages arrived so
    we can push new tiles to the network again.
    This 25 upper limit also seems enough to send all tiles when a whole page is
    requested (e.g. zoom os scroll).
    
    We store the requested tiles in a list, used as a queue. I don't use std::queue
    because sometimes we need to do deduplication (replace older versions of the
    same tile with a newer version).
    
    Change-Id: I22ff3e35c8ded6001c7fc160abdc1f1b12ce3bae

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index ea4d7e95d..1acc06155 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -76,8 +76,6 @@ bool ClientSession::_handleInput(const char *buffer, int length)
     const std::string firstLine = getFirstLine(buffer, length);
     const std::vector<std::string> tokens = LOOLProtocol::tokenize(firstLine.data(), firstLine.size());
 
-    checkTileRequestTimout();
-
     std::shared_ptr<DocumentBroker> docBroker = getDocumentBroker();
     if (!docBroker)
     {
@@ -342,10 +340,6 @@ bool ClientSession::_handleInput(const char *buffer, int length)
         if(iter != _tilesOnFly.end())
             _tilesOnFly.erase(iter);
 
-        if(_tilesOnFly.empty())
-        {
-            _tileCounterStartTime = boost::none;
-        }
         docBroker->sendRequestedTiles(shared_from_this());
         return true;
     }
@@ -1000,25 +994,17 @@ Authorization ClientSession::getAuthorization() const
     return Authorization();
 }
 
-void ClientSession::setTilesOnFly(boost::optional<TileCombined> tiles)
+void ClientSession::addTileOnFly(const TileDesc& tile)
 {
+    std::ostringstream tileID;
+    tileID << tile.getPart() << ":" << tile.getTilePosX() << ":" << tile.getTilePosY() << ":"
+           << tile.getTileWidth() << ":" << tile.getTileHeight();
+    _tilesOnFly.push_back(tileID.str());
+}
 
+void ClientSession::clearTilesOnFly()
+{
     _tilesOnFly.clear();
-    if(tiles == boost::none)
-    {
-        _tileCounterStartTime = boost::none;
-    }
-    else
-    {
-        for (auto& tile : tiles.get().getTiles())
-        {
-            std::ostringstream tileID;
-            tileID << tile.getPart() << ":" << tile.getTilePosX() << ":" << tile.getTilePosY() << ":"
-                   << tile.getTileWidth() << ":" << tile.getTileHeight();
-            _tilesOnFly.push_back(tileID.str());
-        }
-        _tileCounterStartTime = std::chrono::steady_clock::now();
-    }
 }
 
 void ClientSession::onDisconnect()
@@ -1152,17 +1138,4 @@ void ClientSession::handleTileInvalidation(const std::string& message,
     }
 }
 
-void ClientSession::checkTileRequestTimout()
-{
-    if(_tileCounterStartTime != boost::none)
-    {
-        const auto duration = std::chrono::steady_clock::now() - _tileCounterStartTime.get();
-        if( std::chrono::duration_cast<std::chrono::seconds>(duration).count() > 5)
-        {
-            LOG_WRN("Tile request timeout: server waits too long for tileprocessed messages.");
-            _tileCounterStartTime = boost::none;
-        }
-    }
-}
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 2ecb09e8f..5b3e62bc3 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -18,7 +18,7 @@
 #include <Poco/URI.h>
 #include <Rectangle.hpp>
 #include <boost/optional.hpp>
-#include <vector>
+#include <list>
 
 class DocumentBroker;
 
@@ -108,10 +108,11 @@ public:
     /// Set WOPI fileinfo object
     void setWopiFileInfo(std::unique_ptr<WopiStorage::WOPIFileInfo>& wopiFileInfo) { _wopiFileInfo = std::move(wopiFileInfo); }
 
-    boost::optional<TileCombined>& getRequestedTiles() { return _requestedTiles; }
+    boost::optional<std::list<TileDesc>>& getRequestedTiles() { return _requestedTiles; }
 
-    const std::vector<std::string>& getTilesOnFly() const { return _tilesOnFly; }
-    void setTilesOnFly(boost::optional<TileCombined> tiles);
+    void addTileOnFly(const TileDesc& tile);
+    void clearTilesOnFly();
+    size_t getTilesOnFlyCount() const { return _tilesOnFly.size(); }
 
 
 private:
@@ -154,8 +155,6 @@ private:
     void handleTileInvalidation(const std::string& message,
                                 const std::shared_ptr<DocumentBroker>& docBroker);
 
-    void checkTileRequestTimout();
-
 private:
     std::weak_ptr<DocumentBroker> _docBroker;
 
@@ -197,10 +196,9 @@ private:
     // Type of the docuemnt, extracter from status message
     bool _isTextDocument;
 
-    std::vector<std::string> _tilesOnFly;
-    boost::optional<std::chrono::time_point<std::chrono::steady_clock>> _tileCounterStartTime;
+    std::list<std::string> _tilesOnFly;
 
-    boost::optional<TileCombined> _requestedTiles;
+    boost::optional<std::list<TileDesc>> _requestedTiles;
 };
 
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a6707099e..9b176501d 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -41,6 +41,8 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 
+#define TILES_ON_FLY_UPPER_LIMIT 25
+
 using namespace LOOLProtocol;
 
 using Poco::JSON::Object;
@@ -1317,17 +1319,17 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
     }
 
     // Accumulate tiles
-    boost::optional<TileCombined>& requestedTiles = session->getRequestedTiles();
+    boost::optional<std::list<TileDesc>>& requestedTiles = session->getRequestedTiles();
     if(requestedTiles == boost::none)
     {
-        requestedTiles = TileCombined::create(tileCombined.getTiles());
+        requestedTiles = std::list<TileDesc>(tileCombined.getTiles().begin(), tileCombined.getTiles().end());
     }
     // Drop duplicated tiles, but use newer version number
     else
     {
         for (const auto& newTile : tileCombined.getTiles())
         {
-            const TileDesc& firstOldTile = requestedTiles.get().getTiles()[0];
+            const TileDesc& firstOldTile = *(requestedTiles.get().begin());
             if(newTile.getPart() != firstOldTile.getPart() ||
                newTile.getWidth() != firstOldTile.getWidth() ||
                newTile.getHeight() != firstOldTile.getHeight() ||
@@ -1338,7 +1340,7 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
             }
 
             bool tileFound = false;
-            for (auto& oldTile : requestedTiles.get().getTiles())
+            for (auto& oldTile : requestedTiles.get())
             {
                 if(oldTile.getTilePosX() == newTile.getTilePosX() &&
                    oldTile.getTilePosY() == newTile.getTilePosY() )
@@ -1351,7 +1353,7 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
                 }
             }
             if(!tileFound)
-                requestedTiles.get().getTiles().push_back(newTile);
+                requestedTiles.get().push_back(newTile);
         }
     }
 
@@ -1366,15 +1368,16 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
 
     // All tiles were processed on client side what we sent last time, so we can send a new banch of tiles
     // which was invalidated / requested in the meantime
-    boost::optional<TileCombined>& requestedTiles = session->getRequestedTiles();
-    if(session->getTilesOnFly().empty() && requestedTiles != boost::none && !requestedTiles.get().getTiles().empty())
+    boost::optional<std::list<TileDesc>>& requestedTiles = session->getRequestedTiles();
+    if(requestedTiles != boost::none && !requestedTiles.get().empty())
     {
-        session->setTilesOnFly(requestedTiles.get());
-
-        // Satisfy as many tiles from the cache.
         std::vector<TileDesc> tilesNeedsRendering;
-        for (auto& tile : requestedTiles.get().getTiles())
+        while(session->getTilesOnFlyCount() < TILES_ON_FLY_UPPER_LIMIT && !requestedTiles.get().empty())
         {
+            TileDesc& tile = *(requestedTiles.get().begin());
+            session->addTileOnFly(tile);
+
+            // Satisfy as many tiles from the cache.
             std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile);
             if (cachedTile)
             {
@@ -1415,6 +1418,7 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
                 }
                 tileCache().subscribeToTileRendering(tile, session);
             }
+            requestedTiles.get().pop_front();
         }
 
         // Send rendering request for those tiles which were not prerendered
@@ -1425,10 +1429,8 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
             // Forward to child to render.
             const std::string req = newTileCombined.serialize("tilecombine");
             LOG_DBG("Some of the tiles were not prerendered. Sending residual tilecombine: " << req);
-            LOG_DBG("Sending residual tilecombine: " << req);
             _childProcess->sendTextFrame(req);
         }
-        requestedTiles = boost::none;
     }
 }
 
@@ -1437,7 +1439,7 @@ void DocumentBroker::cancelTileRequests(const std::shared_ptr<ClientSession>& se
     std::unique_lock<std::mutex> lock(_mutex);
 
     // Clear tile requests
-    session->setTilesOnFly(boost::none);
+    session->clearTilesOnFly();
     session->getRequestedTiles() = boost::none;
 
     const std::string canceltiles = tileCache().cancelTiles(session);
commit eafb46e09007337eab4ed9207ef14c7e8008ea4d
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
Date:   Thu Jun 21 18:05:54 2018 +0200

    Use a bigger value for this timeout
    
    Change-Id: Ia1afeee72b5310e729b9a6ec86c788d57328619a

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 56d66a053..ea4d7e95d 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1157,7 +1157,7 @@ void ClientSession::checkTileRequestTimout()
     if(_tileCounterStartTime != boost::none)
     {
         const auto duration = std::chrono::steady_clock::now() - _tileCounterStartTime.get();
-        if( std::chrono::duration_cast<std::chrono::seconds>(duration).count() > 2)
+        if( std::chrono::duration_cast<std::chrono::seconds>(duration).count() > 5)
         {
             LOG_WRN("Tile request timeout: server waits too long for tileprocessed messages.");
             _tileCounterStartTime = boost::none;
commit 85581f22fc4577785dda3646e20cee3c84041337
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
Date:   Wed Jun 20 22:17:51 2018 +0200

    Remove this line
    
    Change-Id: I1abd0e200959911fa8b1d4fe3b877bb80ff552bd

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index cb3e03d00..56d66a053 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1160,7 +1160,6 @@ void ClientSession::checkTileRequestTimout()
         if( std::chrono::duration_cast<std::chrono::seconds>(duration).count() > 2)
         {
             LOG_WRN("Tile request timeout: server waits too long for tileprocessed messages.");
-            std::cerr << "Tile request timeout: server waits too long for tileprocessed messages." << std::endl;
             _tileCounterStartTime = boost::none;
         }
     }
commit 7a6d4c78bc5dc2282d33234eddc1d1cc647c53e0
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
Date:   Tue Jun 19 18:47:31 2018 +0200

    Build fix
    
    Change-Id: I23abb2c6f271f95fa89ea12e2364b46c9a9b4389

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7863f7958..a6707099e 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1408,8 +1408,7 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
                 {
                     tile.setVersion(++_tileVersion);
                 }
-                std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
-                if(tileBeingRendered == nullptr)
+                if(!tileCache().hasTileBeingRendered(tile))
                 {
                     tilesNeedsRendering.push_back(tile);
                     _debugRenderedTileCount++;
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index ef697933a..b2d8ca793 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -129,6 +129,11 @@ void TileCache::forgetTileBeingRendered(const TileDesc& tile)
     _tilesBeingRendered.erase(cachedName);
 }
 
+bool TileCache::hasTileBeingRendered(const TileDesc& tile)
+{
+    return findTileBeingRendered(tile) != nullptr;
+}
+
 std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile)
 {
     const std::string fileName = _cacheDir + "/" + cacheFileName(tile);
diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp
index d8c48eaa4..560b66ccc 100644
--- a/wsd/TileCache.hpp
+++ b/wsd/TileCache.hpp
@@ -80,10 +80,13 @@ public:
     void saveLastModified(const Poco::Timestamp& timestamp);
 
     void forgetTileBeingRendered(const TileDesc& tile);
+    bool hasTileBeingRendered(const TileDesc& tile);
 
     void setThreadOwner(const std::thread::id &id) { _owner = id; }
     void assertCorrectThread();
 
+
+
 private:
     void invalidateTiles(int part, int x, int y, int width, int height);
 


More information about the Libreoffice-commits mailing list