[Libreoffice-commits] online.git: 2 commits - kit/Kit.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/TileCache.cpp wsd/TileCache.hpp

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Tue Jul 24 18:54:23 UTC 2018


 kit/Kit.cpp            |    4 +++-
 wsd/ClientSession.cpp  |    6 +++++-
 wsd/ClientSession.hpp  |   23 ++++++++++++++++++++++-
 wsd/DocumentBroker.cpp |   39 ++++++++++++---------------------------
 wsd/TileCache.cpp      |   35 ++++++++++++++++++++++++++++-------
 wsd/TileCache.hpp      |    2 +-
 6 files changed, 71 insertions(+), 38 deletions(-)

New commits:
commit c2a5f6acb0f1e93f19104b761661c852d930fb9e
Author:     Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Mon Jul 23 16:09:55 2018 +0200
Commit:     Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Tue Jul 24 20:52:53 2018 +0200

    Store number of tiles sent to kit for rendering
    
    and use that info also to avoid sending to much tiles on the network.
    
    Change-Id: Iab2d7af64693047a3c1cfe9f73de80a7100bbc13

diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 47cedaa8d..a6e386243 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1079,7 +1079,9 @@ public:
                 // The tile content is identical to what the client already has, so skip it
                 LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," <<
                         positionY << ") oldhash==hash (" << hash << "), wireId: " << wireId << " skipping");
-                tiles.erase(tiles.begin() + tileIndex);
+                tiles[tileIndex].setWireId(wireId);
+                tiles[tileIndex].setImgSize(0);
+                tileIndex++;
                 continue;
             }
 
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 41fa32193..b421d3304 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -56,7 +56,9 @@ ClientSession::ClientSession(const std::string& id,
     _tileHeightPixel(0),
     _tileWidthTwips(0),
     _tileHeightTwips(0),
-    _isTextDocument(false)
+    _isTextDocument(false),
+    _tilesOnFly(0),
+    _tilesBeingRendered(0)
 {
     assert(!creatingPngThumbnail || thumbnailFile != "");
     const size_t curConnections = ++LOOLWSD::NumConnections;
@@ -350,6 +352,8 @@ bool ClientSession::_handleInput(const char *buffer, int length)
         auto iter = std::find(_tilesOnFly.begin(), _tilesOnFly.end(), tileID);
         if(iter != _tilesOnFly.end())
             _tilesOnFly.erase(iter);
+        else
+            LOG_WRN("Tileprocessed message with an unknown tile ID");
 
         docBroker->sendRequestedTiles(shared_from_this());
         return true;
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index ca38257c6..fe565a672 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -137,6 +137,12 @@ public:
     /// This method updates internal data related to sent tiles (wireID and tiles-on-fly)
     /// Call this method anytime when a new tile is sent to the client
     void traceTileBySend(const TileDesc& tile);
+
+    void traceSubscribe() { ++_tilesBeingRendered; }
+    void traceUnSubscribe() { --_tilesBeingRendered; }
+    void clearSubscription() { _tilesBeingRendered = 0; }
+
+    int getTilesBeingRendered() const {return _tilesBeingRendered;}
 private:
 
     /// SocketHandler: disconnection event.
@@ -236,6 +242,10 @@ private:
     /// TileID's of the sent tiles. Push by sending and pop by tileprocessed message from the client.
     std::list<std::string> _tilesOnFly;
 
+    /// Number of tiles requested from kit, which this session is subsrcibed to
+    /// Track only non-thumbnail tiles (getId() == -1)
+    int _tilesBeingRendered;
+
     /// Requested tiles are stored in this list, before we can send them to the client
     boost::optional<std::list<TileDesc>> _requestedTiles;
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7c1e30e2b..84ad69a15 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1382,7 +1382,8 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
     if(requestedTiles != boost::none && !requestedTiles.get().empty())
     {
         std::vector<TileDesc> tilesNeedsRendering;
-        while(session->getTilesOnFlyCount() < tilesOnFlyUpperLimit && !requestedTiles.get().empty())
+        while(session->getTilesOnFlyCount() + session->getTilesBeingRendered() < tilesOnFlyUpperLimit
+              && !requestedTiles.get().empty())
         {
             TileDesc& tile = *(requestedTiles.get().begin());
 
@@ -1416,15 +1417,9 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
             else
             {
                 // Not cached, needs rendering.
-                if(tile.getVersion() == -1) // Rendering of this tile was not requested yet
-                {
-                    tile.setVersion(++_tileVersion);
-                }
-                if(!tileCache().hasTileBeingRendered(tile))
-                {
-                    tilesNeedsRendering.push_back(tile);
-                    _debugRenderedTileCount++;
-                }
+                tile.setVersion(++_tileVersion);
+                tilesNeedsRendering.push_back(tile);
+                _debugRenderedTileCount++;
                 tileCache().subscribeToTileRendering(tile, session);
             }
             requestedTiles.get().pop_front();
@@ -1496,25 +1491,16 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
 
     try
     {
-        const size_t length = payload.size();
-        if (firstLine.size() < static_cast<std::string::size_type>(length) - 1)
-        {
-            const TileCombined tileCombined = TileCombined::parse(firstLine);
-            const char* buffer = payload.data();
-            size_t offset = firstLine.size() + 1;
+        const TileCombined tileCombined = TileCombined::parse(firstLine);
+        const char* buffer = payload.data();
+        size_t offset = firstLine.size() + 1;
 
-            std::unique_lock<std::mutex> lock(_mutex);
+        std::unique_lock<std::mutex> lock(_mutex);
 
-            for (const auto& tile : tileCombined.getTiles())
-            {
-                tileCache().saveTileAndNotify(tile, buffer + offset, tile.getImgSize());
-                offset += tile.getImgSize();
-            }
-        }
-        else
+        for (const auto& tile : tileCombined.getTiles())
         {
-            LOG_WRN("Dropping empty tilecombine response: " << firstLine);
-            // They will get re-issued if we don't forget them.
+            tileCache().saveTileAndNotify(tile, buffer + offset, tile.getImgSize());
+            offset += tile.getImgSize();
         }
     }
     catch (const std::exception& exc)
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 2856b306c..f80f64a1c 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -120,13 +120,20 @@ std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(c
     return tile != _tilesBeingRendered.end() ? tile->second : nullptr;
 }
 
-void TileCache::forgetTileBeingRendered(const TileDesc& tile)
+void TileCache::forgetTileBeingRendered(std::shared_ptr<TileCache::TileBeingRendered> tileBeingRendered, const TileDesc& tile)
 {
-    const std::string cachedName = cacheFileName(tile);
-
     assertCorrectThread();
+    assert(tileBeingRendered);
+    assert(_tilesBeingRendered.find(tileBeingRendered->getCacheName()) != _tilesBeingRendered.end());
 
-    _tilesBeingRendered.erase(cachedName);
+    for(auto& subscriber : tileBeingRendered->_subscribers)
+    {
+        std::shared_ptr<ClientSession> session = subscriber.lock();
+        if(session && tile.getId() == -1)
+            session->traceUnSubscribe();
+    }
+
+    _tilesBeingRendered.erase(tileBeingRendered->getCacheName());
 }
 
 bool TileCache::hasTileBeingRendered(const TileDesc& tile)
@@ -158,6 +165,17 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
 
     std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
 
+    // Kit did not send image data, because tile has the same wireID as the previously sent tile
+    // We need to remove only the subscriptions
+    if(size == 0)
+    {
+        if(tileBeingRendered && tileBeingRendered->getVersion() <= tile.getVersion())
+        {
+            forgetTileBeingRendered(tileBeingRendered, tile);
+        }
+        return;
+    }
+
     // Save to disk.
     const std::string cachedName = (tileBeingRendered ? tileBeingRendered->getCacheName()
                                                : cacheFileName(tile));
@@ -226,7 +244,7 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
         {
             LOG_DBG("STATISTICS: tile " << tile.getVersion() << " internal roundtrip " <<
                     tileBeingRendered->getElapsedTimeMs() << " ms.");
-            _tilesBeingRendered.erase(cachedName);
+            forgetTileBeingRendered(tileBeingRendered, tile);
         }
     }
     else
@@ -477,6 +495,8 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
         LOG_DBG("Subscribing " << subscriber->getName() << " to tile " << name << " which has " <<
                 tileBeingRendered->_subscribers.size() << " subscribers already.");
         tileBeingRendered->_subscribers.push_back(subscriber);
+        if(tile.getId() == -1)
+            subscriber->traceSubscribe();
 
         const auto duration = (std::chrono::steady_clock::now() - tileBeingRendered->getStartTime());
         if (std::chrono::duration_cast<std::chrono::milliseconds>(duration).count() > COMMAND_TIMEOUT_MS)
@@ -496,6 +516,8 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
 
         tileBeingRendered = std::make_shared<TileBeingRendered>(cachedName, tile);
         tileBeingRendered->_subscribers.push_back(subscriber);
+        if(tile.getId() == -1)
+            subscriber->traceSubscribe();
         _tilesBeingRendered[cachedName] = tileBeingRendered;
     }
 }
@@ -542,6 +564,7 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri
         ++it;
     }
 
+    subscriber->clearSubscription();
     const std::string canceltiles = oss.str();
     return canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles;
 }
diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp
index 560b66ccc..4fc32edea 100644
--- a/wsd/TileCache.hpp
+++ b/wsd/TileCache.hpp
@@ -79,7 +79,7 @@ public:
     /// Store the timestamp to modtime.txt.
     void saveLastModified(const Poco::Timestamp& timestamp);
 
-    void forgetTileBeingRendered(const TileDesc& tile);
+    void forgetTileBeingRendered(std::shared_ptr<TileCache::TileBeingRendered> tileBeingRendered, const TileDesc& tile);
     bool hasTileBeingRendered(const TileDesc& tile);
 
     void setThreadOwner(const std::thread::id &id) { _owner = id; }
commit b014804ce2229030a8816eb3c35a08f9af9e4676
Author:     Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Mon Jul 23 16:11:47 2018 +0200
Commit:     Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Tue Jul 24 20:52:45 2018 +0200

    Trace sent tiles when they are actually sent
    
    SenderQueue might drop some tiles, so we were waiting for
    tileprocessed message for a tile which was not sent at all.
    
    Change-Id: I7c502966f656e46df7c22002dee19aeabbf97774

diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 1b21425d4..ca38257c6 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -86,7 +86,18 @@ public:
             docBroker->assertCorrectThread();
 
         LOG_TRC(getName() << " enqueueing client message " << data->id());
-        _senderQueue.enqueue(data);
+        size_t sizeBefore = _senderQueue.size();
+        size_t newSize = _senderQueue.enqueue(data);
+        if(sizeBefore != newSize)
+        {
+            // Track sent tile
+            const std::string command = data->firstToken();
+            if (command == "tile:")
+            {
+                const TileDesc tile = TileDesc::parse(data->firstLine());
+                traceTileBySend(tile);
+            }
+        }
     }
 
     /// Set the save-as socket which is used to send convert-to results.
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7eead269b..7c1e30e2b 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1411,7 +1411,6 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
                 cachedTile->read(output.data() + pos, size);
                 cachedTile->close();
 
-                session->traceTileBySend(tile);
                 session->sendBinaryFrame(output.data(), output.size());
             }
             else
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 84dd5b3bd..2856b306c 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -190,7 +190,6 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
             std::shared_ptr<ClientSession> firstSession = firstSubscriber.lock();
             if (firstSession)
             {
-                firstSession->traceTileBySend(tile);
                 firstSession->enqueueSendMessage(payload);
             }
 
@@ -212,7 +211,6 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
                     std::shared_ptr<ClientSession> session = subscriber.lock();
                     if (session)
                     {
-                        session->traceTileBySend(tile);
                         session->enqueueSendMessage(payload);
                     }
                 }


More information about the Libreoffice-commits mailing list