common/RenderTiles.hpp | 12 +++++-
wsd/ClientSession.cpp | 5 +-
wsd/DocumentBroker.cpp | 16 +++++----
wsd/TileCache.cpp | 85 +++++++++++++++++++++++++++----------------------
wsd/TileCache.hpp | 13 +++----
wsd/TileDesc.hpp | 8 ++++
6 files changed, 82 insertions(+), 57 deletions(-)
New commits:
commit 4c6ba6d85096a223b5c6672ee2f4b51c12ca41c3
Author: Michael Meeks <michael.me...@collabora.com>
AuthorDate: Fri Aug 7 18:37:53 2020 +0100
Commit: Michael Meeks <michael.me...@collabora.com>
CommitDate: Fri Aug 7 20:01:40 2020 +0200
Notify WSD of tiles which we didn't need to render.
When we get a wid match, this helps WSD to cleanup its tile
subscriber list effectively.
Change-Id: I6517039fb3d8c9ad8f53aef549b8adbb79961ce1
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100348
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Michael Meeks <michael.me...@collabora.com>
diff --git a/common/RenderTiles.hpp b/common/RenderTiles.hpp
index 011503f12..c5bc9d03e 100644
--- a/common/RenderTiles.hpp
+++ b/common/RenderTiles.hpp
@@ -638,6 +638,9 @@ namespace RenderTiles
// 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");
+ // Push a zero byte image to inform WSD we didn't need that.
+ // This allows WSD side TileCache to free up waiting
subscribers.
+ pushRendered(renderedTiles, tiles[tileIndex], wireId, 0);
tileIndex++;
continue;
}
@@ -701,13 +704,16 @@ namespace RenderTiles
tileIndex++;
}
+ // empty ones come first
+ size_t zeroCheckStart = renderedTiles.size();
+
pngPool.run();
- for (auto &i : renderedTiles)
+ for (size_t i = zeroCheckStart; i < renderedTiles.size(); ++i)
{
- if (i.getImgSize() == 0)
+ if (renderedTiles[i].getImgSize() == 0)
{
- LOG_ERR("Encoded 0-sized tile!");
+ LOG_TRC("Encoded 0-sized tile in slot !" << i);
assert(!"0-sized tile enocded!");
}
}
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 5120bb099..a5e62fcc4 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -2034,7 +2034,7 @@ 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)
+ if (firstLine.size() <= static_cast<std::string::size_type>(length) -
1)
{
const TileCombined tileCombined = TileCombined::parse(firstLine);
const char* buffer = payload.data();
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index e1cdefe8d..982cd985b 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -175,19 +175,24 @@ void TileCache::saveTileAndNotify(const TileDesc& tile,
const char *data, const
{
assertCorrectThread();
- // Save to disk.
+ if (size > 0)
+ {
+ // Save to in-memory cache.
- // Ignore if we can't save the tile, things will work anyway, but slower.
- // An error indication is supposed to be sent to all users in that case.
- saveDataToCache(tile, data, size);
- LOG_TRC("Saved cache tile: " << cacheFileName(tile) << " of size " << size
<< " bytes");
+ // Ignore if we can't save the tile, things will work anyway, but
slower.
+ // An error indication is supposed to be sent to all users in that
case.
+ saveDataToCache(tile, data, size);
+ LOG_TRC("Saved cache tile: " << cacheFileName(tile) << " of size " <<
size << " bytes");
+ }
+ else
+ LOG_TRC("Zero sized cache tile: " << cacheFileName(tile));
// Notify subscribers, if any.
std::shared_ptr<TileBeingRendered> tileBeingRendered =
findTileBeingRendered(tile);
if (tileBeingRendered)
{
const size_t subscriberCount =
tileBeingRendered->getSubscribers().size();
- if (subscriberCount > 0)
+ if (size > 0 && subscriberCount > 0)
{
std::string response = tile.serialize("tile:");
LOG_DBG("Sending tile message to " << subscriberCount << "
subscribers: " << response);
@@ -229,10 +234,9 @@ void TileCache::saveTileAndNotify(const TileDesc& tile,
const char *data, const
}
}
}
- else
- {
+ else if (subscriberCount == 0)
LOG_DBG("No subscribers for: " << cacheFileName(tile));
- }
+ // else zero sized
// Remove subscriptions.
if (tileBeingRendered->getVersion() <= tile.getVersion())
@@ -243,9 +247,7 @@ void TileCache::saveTileAndNotify(const TileDesc& tile,
const char *data, const
}
}
else
- {
LOG_DBG("No subscribers for: " << cacheFileName(tile));
- }
}
bool TileCache::getTextStream(StreamType type, const std::string& fileName,
std::string& content)
commit 1061ac90ce212a6b0376102c42b763ba3d6c75d1
Author: Michael Meeks <michael.me...@collabora.com>
AuthorDate: Fri Aug 7 17:36:56 2020 +0100
Commit: Michael Meeks <michael.me...@collabora.com>
CommitDate: Fri Aug 7 20:01:30 2020 +0200
TileCache: cleanup debug, propagate now more helpfully & fix staleness.
Stale tiles were still being counted, unhelpfully. Avoid doing lots
of ::now() calls, and yet detect this.
Change-Id: Ib1e4b2f1968c1994849bb23ec54e28f6706230ee
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100347
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.me...@collabora.com>
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index baaab79f7..a3fa8e078 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1567,9 +1567,8 @@ bool ClientSession::forwardToClient(const
std::shared_ptr<Message>& payload)
void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& data)
{
const std::shared_ptr<DocumentBroker> docBroker = _docBroker.lock();
- // If in the correct thread - no need for wakeups.
- if (docBroker)
- docBroker->assertCorrectThread();
+ LOG_CHECK_RET(docBroker && "Null DocumentBroker instance", );
+ docBroker->assertCorrectThread();
const std::string command = data->firstToken();
std::unique_ptr<TileDesc> tile;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 98d2c7614..5120bb099 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1700,6 +1700,7 @@ size_t DocumentBroker::getMemorySize() const
_sessions.size() * sizeof(ClientSession);
}
+// Expected to be legacy, ~all new requests are tilecombinedRequests
void DocumentBroker::handleTileRequest(TileDesc& tile,
const std::shared_ptr<ClientSession>&
session)
{
@@ -1718,17 +1719,18 @@ void DocumentBroker::handleTileRequest(TileDesc& tile,
return;
}
+ auto now = std::chrono::steady_clock::now();
if (tile.getBroadcast())
{
for (auto& it: _sessions)
{
if (!it.second->inWaitDisconnected())
- tileCache().subscribeToTileRendering(tile, it.second);
+ tileCache().subscribeToTileRendering(tile, it.second, now);
}
}
else
{
- tileCache().subscribeToTileRendering(tile, session);
+ tileCache().subscribeToTileRendering(tile, session, now);
}
// Forward to child to render.
@@ -1907,6 +1909,8 @@ void DocumentBroker::sendRequestedTiles(const
std::shared_ptr<ClientSession>& se
// Drop tiles which we are waiting for too long
session->removeOutdatedTilesOnFly();
+ auto now = std::chrono::steady_clock::now();
+
// All tiles were processed on client side that we sent last time, so we
can send
// a new batch of tiles which was invalidated / requested in the meantime
std::deque<TileDesc>& requestedTiles = session->getRequestedTiles();
@@ -1914,7 +1918,7 @@ void DocumentBroker::sendRequestedTiles(const
std::shared_ptr<ClientSession>& se
{
size_t delayedTiles = 0;
std::vector<TileDesc> tilesNeedsRendering;
- size_t beingRendered =
_tileCache->countTilesBeingRenderedForSession(session);
+ size_t beingRendered =
_tileCache->countTilesBeingRenderedForSession(session, now);
while (session->getTilesOnFlyCount() + beingRendered <
tilesOnFlyUpperLimit &&
!requestedTiles.empty() &&
// If we delayed all tiles we don't send any tile (we will when
next tileprocessed message arrives)
@@ -1944,14 +1948,14 @@ void DocumentBroker::sendRequestedTiles(const
std::shared_ptr<ClientSession>& se
else
{
// Not cached, needs rendering.
- if (!tileCache().hasTileBeingRendered(tile) || // There is no
in progress rendering of the given tile
+ if (!tileCache().hasTileBeingRendered(tile, &now) || // There
is no in progress rendering of the given tile
tileCache().getTileBeingRenderedVersion(tile) <
tile.getVersion()) // We need a newer version
{
tile.setVersion(++_tileVersion);
tilesNeedsRendering.push_back(tile);
_debugRenderedTileCount++;
}
- tileCache().subscribeToTileRendering(tile, session);
+ tileCache().subscribeToTileRendering(tile, session, now);
beingRendered++;
}
requestedTiles.pop_front();
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 8be7b9aff..e1cdefe8d 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -70,19 +70,19 @@ void TileCache::clear()
/// rendering latency.
struct TileCache::TileBeingRendered
{
- explicit TileBeingRendered(const TileDesc& tile)
- : _startTime(std::chrono::steady_clock::now())
- , _tile(tile)
- {
- }
+ explicit TileBeingRendered(const TileDesc& tile, const
std::chrono::steady_clock::time_point &now)
+ : _startTime(now), _tile(tile) { }
const TileDesc& getTile() const { return _tile; }
int getVersion() const { return _tile.getVersion(); }
void setVersion(int version) { _tile.setVersion(version); }
std::chrono::steady_clock::time_point getStartTime() const { return
_startTime; }
- double getElapsedTimeMs() const { return
std::chrono::duration_cast<std::chrono::milliseconds>
-
(std::chrono::steady_clock::now() - _startTime).count(); }
+ double getElapsedTimeMs(const std::chrono::steady_clock::time_point *now =
nullptr) const
+ { return std::chrono::duration_cast<std::chrono::milliseconds>
+ ((now ? *now : std::chrono::steady_clock::now()) -
_startTime).count(); }
+ bool isStale(const std::chrono::steady_clock::time_point *now = nullptr)
const
+ { return getElapsedTimeMs(now) > COMMAND_TIMEOUT_MS; }
std::vector<std::weak_ptr<ClientSession>>& getSubscribers() { return
_subscribers; }
void dumpState(std::ostream& os);
@@ -93,11 +93,15 @@ private:
TileDesc _tile;
};
-size_t TileCache::countTilesBeingRenderedForSession(const
std::shared_ptr<ClientSession>& session)
+size_t TileCache::countTilesBeingRenderedForSession(const
std::shared_ptr<ClientSession>& session,
+ const
std::chrono::steady_clock::time_point &now)
{
size_t count = 0;
for (auto& it : _tilesBeingRendered)
{
+ if (it.second->isStale(&now))
+ continue;
+
for (auto& s : it.second->getSubscribers())
{
if (s.lock() == session)
@@ -108,6 +112,16 @@ size_t TileCache::countTilesBeingRenderedForSession(const
std::shared_ptr<Client
return count;
}
+bool TileCache::hasTileBeingRendered(const TileDesc& tileDesc, const
std::chrono::steady_clock::time_point *now) const
+{
+ const auto it = _tilesBeingRendered.find(tileDesc);
+ if (it == _tilesBeingRendered.end())
+ return false;
+
+ /// did we stall ? if so re-issue.
+ return !now ? true : !it->second->isStale(now);
+}
+
std::shared_ptr<TileCache::TileBeingRendered>
TileCache::findTileBeingRendered(const TileDesc& tileDesc)
{
assertCorrectThread();
@@ -397,47 +411,40 @@ bool TileCache::intersectsTile(const TileDesc &tileDesc,
int part, int x, int y,
}
// FIXME: to be further simplified when we centralize tile messages.
-void TileCache::subscribeToTileRendering(const TileDesc& tile, const
std::shared_ptr<ClientSession>& subscriber)
+void TileCache::subscribeToTileRendering(const TileDesc& tile, const
std::shared_ptr<ClientSession>& subscriber,
+ const
std::chrono::steady_clock::time_point &now)
{
- std::ostringstream oss;
- oss << '(' << tile.getNormalizedViewId() << ',' << tile.getPart() << ','
<< tile.getTilePosX() << ',' << tile.getTilePosY() << ')';
- const std::string name = oss.str();
-
assertCorrectThread();
std::shared_ptr<TileBeingRendered> tileBeingRendered =
findTileBeingRendered(tile);
if (tileBeingRendered)
{
+ if (tileBeingRendered->isStale(&now))
+ LOG_DBG("Painting stalled; need to re-issue on tile " <<
tile.debugName());
+
for (const auto &s : tileBeingRendered->getSubscribers())
{
if (s.lock().get() == subscriber.get())
{
- LOG_DBG("Redundant request to subscribe on tile " << name);
+ LOG_DBG("Redundant request to subscribe on tile " <<
tile.debugName());
tileBeingRendered->setVersion(tile.getVersion());
return;
}
}
- LOG_DBG("Subscribing " << subscriber->getName() << " to tile " << name
<< " which has " <<
+ LOG_DBG("Subscribing " << subscriber->getName() << " to tile " <<
tile.debugName() << " which has " <<
tileBeingRendered->getSubscribers().size() << " subscribers
already.");
tileBeingRendered->getSubscribers().push_back(subscriber);
-
- const auto duration = (std::chrono::steady_clock::now() -
tileBeingRendered->getStartTime());
- if
(std::chrono::duration_cast<std::chrono::milliseconds>(duration).count() >
COMMAND_TIMEOUT_MS)
- {
- // Tile painting has stalled. Reissue.
- tileBeingRendered->setVersion(tile.getVersion());
- }
}
else
{
- LOG_DBG("Subscribing " << subscriber->getName() << " to tile " << name
<<
+ LOG_DBG("Subscribing " << subscriber->getName() << " to tile " <<
tile.debugName() <<
" ver=" << tile.getVersion() << " which has no subscribers "
<< tile.serialize());
assert(_tilesBeingRendered.find(tile) == _tilesBeingRendered.end());
- tileBeingRendered = std::make_shared<TileBeingRendered>(tile);
+ tileBeingRendered = std::make_shared<TileBeingRendered>(tile, now);
tileBeingRendered->getSubscribers().push_back(subscriber);
_tilesBeingRendered[tile] = tileBeingRendered;
}
@@ -446,10 +453,10 @@ void TileCache::subscribeToTileRendering(const TileDesc&
tile, const std::shared
void TileCache::registerTileBeingRendered(const TileDesc& tile)
{
std::shared_ptr<TileBeingRendered> tileBeingRendered =
findTileBeingRendered(tile);
+ auto now = std::chrono::steady_clock::now();
if (tileBeingRendered)
{
- const auto duration = (std::chrono::steady_clock::now() -
tileBeingRendered->getStartTime());
- if
(std::chrono::duration_cast<std::chrono::milliseconds>(duration).count() >
COMMAND_TIMEOUT_MS)
+ if (tileBeingRendered->isStale(&now))
{
// Tile painting has stalled. Reissue.
tileBeingRendered->setVersion(tile.getVersion());
@@ -459,7 +466,7 @@ void TileCache::registerTileBeingRendered(const TileDesc&
tile)
{
assert(_tilesBeingRendered.find(tile) == _tilesBeingRendered.end());
- tileBeingRendered = std::make_shared<TileBeingRendered>(tile);
+ tileBeingRendered = std::make_shared<TileBeingRendered>(tile, now);
_tilesBeingRendered[tile] = tileBeingRendered;
}
}
diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp
index 04c3b8e17..8044919df 100644
--- a/wsd/TileCache.hpp
+++ b/wsd/TileCache.hpp
@@ -81,7 +81,8 @@ public:
/// Subscribes if no subscription exists and returns the version number.
/// Otherwise returns 0 to signify a subscription exists.
- void subscribeToTileRendering(const TileDesc& tile, const
std::shared_ptr<ClientSession>& subscriber);
+ void subscribeToTileRendering(const TileDesc& tile, const
std::shared_ptr<ClientSession>& subscriber,
+ const std::chrono::steady_clock::time_point&
now);
/// Create the TileBeingRendered object for the given tile indicating that
the tile was sent to
/// the kit for rendering. Note: subscribeToTileRendering calls this
internally, so you don't need
@@ -126,13 +127,11 @@ public:
void forgetTileBeingRendered(const
std::shared_ptr<TileCache::TileBeingRendered>& tileBeingRendered);
double getTileBeingRenderedElapsedTimeMs(const TileDesc &tileDesc) const;
- size_t countTilesBeingRenderedForSession(const
std::shared_ptr<ClientSession>& session);
- inline bool hasTileBeingRendered(const TileDesc& tileDesc) const
- {
- return _tilesBeingRendered.find(tileDesc) != _tilesBeingRendered.end();
- }
+ size_t countTilesBeingRenderedForSession(const
std::shared_ptr<ClientSession>& session,
+ const
std::chrono::steady_clock::time_point& now);
+ bool hasTileBeingRendered(const TileDesc& tileDesc, const
std::chrono::steady_clock::time_point *now = nullptr) const;
- int getTileBeingRenderedVersion(const TileDesc& tileDesc);
+ int getTileBeingRenderedVersion(const TileDesc& tileDesc);
/// Set the high watermark for tilecache size
void setMaxCacheSize(size_t cacheSize);
diff --git a/wsd/TileDesc.hpp b/wsd/TileDesc.hpp
index cec900d50..28ecc4256 100644
--- a/wsd/TileDesc.hpp
+++ b/wsd/TileDesc.hpp
@@ -193,6 +193,14 @@ public:
return oss.str();
}
+ /// short name for a tile for debugging.
+ std::string debugName() const
+ {
+ std::ostringstream oss;
+ oss << '(' << getNormalizedViewId() << ',' << getPart() << ',' <<
getTilePosX() << ',' << getTilePosY() << ')';
+ return oss.str();
+ }
+
/// Deserialize a TileDesc from a tokenized string.
static TileDesc parse(const StringVector& tokens)
{
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
